diff --git a/AGENTS.md b/AGENTS.md index 7a42a2a..985b6ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,12 +60,16 @@ If a tracker concept (issue number, login, HTTP call, label color, `sub_url`, - `scripts/issue_ac.py` — list the body's checkboxes; tick one by number or substring, changing exactly one character of the file - `scripts/issue_tree.py` — draw the dependency graph + - `scripts/issue_evict.py` — remove closed issues from the store; never an + `origin: local` one - `scripts/issue_index.py` — rebuild `tmp/issues/INDEX.md` - `skills/sync` — move issues between the local store and Gitea (`/tea:sync`) - `scripts/map.py` — md ↔ Gitea JSON, pure, no I/O; label colors live here - `scripts/_gitea.py` — transport: login pin, `tea api`, pagination, filters, label ids, the remote-id map - `scripts/pull.py`, `push.py`, `remote.py`, `comment.py` + - `scripts/evict.py` — refresh `state:` from Gitea, then hand the decision to + the domain's `issue_evict.run` - `skills/page` — a discussion's artifacts as a page tree (`/tea:page`), entirely offline - `references/pages.md` — canonical page-tree format; single source of truth @@ -151,7 +155,29 @@ line so plain grep works without a parser. - `.remote.json` is therefore no longer "an index over the files": it is the local number → slug ledger, its entries outlive the files they name, and nothing prunes them. It is still recoverable — from the markers in Gitea, not - from the files. + from the files. **Eviction does not prune it either**, for the same reason a + push does not: an evicted issue is in exactly the state a pushed one is. +- **A closed issue is evicted, not archived.** `issue_evict.py` removes + `.md` and every sidecar under that slug for anything that is `state: + closed` **and** carries an `origin:` naming a tracker, then rebuilds + `INDEX.md`. `--dry-run` prints and writes nothing. **`origin: local` is never + evicted, in any state, not even when named on the command line** — that file + *is* the issue and nothing can fetch it back. +- **Eviction lives in the domain** (`skills/issue/scripts/issue_evict.py`), + because its two inputs — `state:` and `origin:` — are domain fields and the + answer is already on disk. No network, no login, no `tea`. + `skills/sync/scripts/evict.py` is the bridge form: it refreshes `state:` from + the tracker first (a local `state:` is only as fresh as the last pull) and then + calls `issue_evict.run`. One implementation of "what may be evicted", in the + layer that owns the fields it reads. Same gate as push, one step earlier: a + failed or unconfirmed tracker answer evicts nothing at all. +- **Pull by number fetches an issue in any state — a number is a number.** An + address is not a query: `pull.py 42` puts a closed issue on disk exactly as it + always has, and so does `#42`, `owner/repo#42`, or its URL. Only filter mode + (`--milestone`, `--label`, `-q`) leaves closed issues out. Eviction does not + revoke this: a closed issue pulled after a cleanup lands on disk again, and + that is the tracker answering what it was asked, not a regression. Evict it + again when you are done with it. - Pulling overwrites the body — a fetch, not a merge. It is also how a pushed issue comes back at all. - No drift tracking, and now nothing to track: there is no second copy to diff --git a/agents/tea-runner.md b/agents/tea-runner.md index 741f42c..3931724 100644 --- a/agents/tea-runner.md +++ b/agents/tea-runner.md @@ -29,9 +29,10 @@ to fill the gap yourself. Load the skill, do not remember the flags: -- `/tea:sync` — `pull.py`, `push.py`, `comment.py`, `remote.py`, `labels.py` +- `/tea:sync` — `pull.py`, `push.py`, `comment.py`, `remote.py`, `labels.py`, + `evict.py` - `/tea:issue` — `issue_check.py`, `issue_tree.py`, `issue_index.py`, - `issue_new.py`, `issue_ac.py` + `issue_new.py`, `issue_ac.py`, `issue_evict.py` - `/tea:wiki` — `wiki_ls.py`, `wiki_pull.py`, `wiki_push.py` - `/tea:page` — `page_import.py`, `page_index.py`, `page_ls.py` @@ -67,8 +68,13 @@ instead of trying it. the issue has. 4. **Do not close, delete, or retitle anything** on either side. On the wiki that means no `--retitle`: renaming a published page abandons the old one. - The one deletion you may cause is push's own, on the issue you were told to - push. + Two deletions are allowed, both local and both only when the caller asked for + them: push's own, on the issue you were told to push, and eviction + (`issue_evict.py` / `evict.py`) of closed issues. Run eviction with + `--dry-run` first and report what it named; never widen the set past what the + caller said. It refuses to touch an `origin: local` issue by itself — that is + the script's guarantee, not your judgement, and it is not a reason to point + it at a store nobody asked you to clean. 5. **One retry, maximum.** A command that fails twice is a finding. Do not permute flags looking for one that works. 6. **No payload dumps.** Never run `tea issues -o json`, never `cat` a pulled diff --git a/skills/issue/SKILL.md b/skills/issue/SKILL.md index 03e2f1e..8c6f917 100644 --- a/skills/issue/SKILL.md +++ b/skills/issue/SKILL.md @@ -38,6 +38,7 @@ All offline, all in `/scripts/`. | `issue_check.py [id…]` | validate against the canonical format; exit 1 on errors | | `issue_ac.py [--check N\|TEXT]` | list the body's checkboxes; tick or untick one | | `issue_tree.py [id…]` | draw the dependency graph from `depends:` | +| `issue_evict.py [id…] [--dry-run]` | remove closed issues from the store; **never** an `origin: local` one | | `issue_index.py` | rebuild `tmp/issues/INDEX.md` | | `issue.py` | the domain module the others import — not a command | @@ -205,6 +206,50 @@ on `tmp/issues/.md`, and this layer does not know the difference. Getting the rewritten body into the tracker is a separate decision — `push.py --update` in `/tea:sync` — and is no part of this. +## Evicting closed issues + +The store is a working set, not an archive. A closed issue is not a unit of +work any more, and one command takes it out — no `rm`, no rebuilding `INDEX.md` +by hand: + +```bash +python3 /scripts/issue_evict.py --dry-run # what would go +python3 /scripts/issue_evict.py # every closed one +python3 /scripts/issue_evict.py old-thing # just this one +``` + +Two conditions, both read off the file, and the second one is the whole safety +argument: + +| `state:` | `origin:` | what eviction does | +|---|---|---| +| `closed` | a tracker | removes `.md` and every sidecar under that slug | +| `closed` | `local` | **keeps it, always**, and says why | +| `open` | anything | keeps it | + +**`origin: local` is never evicted, in any state, not even when you name it on +the command line.** That file *is* the issue; there is no copy to fetch back. +Only a file whose own metadata says the work lives somewhere else may go — the +same trade `push.py` makes when it drops a file the tracker just confirmed. + +- `--dry-run` prints what would go and writes nothing at all, `INDEX.md` + included. +- `INDEX.md` is rebuilt afterwards, so the table and the directory agree. It is + rebuilt only when something was actually removed. +- `.remote.json` is **not** pruned, deliberately: it is the number → slug + ledger, and its entries are supposed to outlive the files they name (that is + what makes `pull.py ` land on the same slug after a push). An evicted issue + is in exactly the state a pushed one is. +- **This is not a one-off migration.** `pull.py ` fetches an issue in any + state — a number is an address, not a query — so a closed issue pulled after + an eviction lands on disk again. Not a regression: evict it again when you are + done reading it. + +This command is offline and decides from `state:` in the file, which is only as +fresh as the last pull. To have the tracker's answer instead — an issue closed +in the web UI five minutes ago — use `/tea:sync`'s `evict.py`, which refreshes +`state:` first and then calls exactly this decision. + ## Dependency graph `depends:` is the authoritative edge list; the body's `## Depends on` section diff --git a/skills/issue/references/format.md b/skills/issue/references/format.md index 3b74b42..6869e94 100644 --- a/skills/issue/references/format.md +++ b/skills/issue/references/format.md @@ -82,19 +82,24 @@ represent a local issue and a synced one without a second format. leaves this machine is valid and finished work; pushing it is optional and nothing here treats it as a draft. -It is not a *permanent* state, and this is the one place where the file's fate -depends on it: +It is not a *permanent* state, and it is what the file's fate depends on: -| `origin:` | what the file is | what a push does to it | -|---|---|---| -| `local` | the issue itself — the only copy there is | creates it in the tracker, then deletes the file | -| a tracker | a working copy of something the tracker already has | updates the tracker, then deletes the file | +| `origin:` | what the file is | what a push does to it | what eviction does to it | +|---|---|---|---| +| `local` | the issue itself — the only copy there is | creates it in the tracker, then deletes the file | **nothing, ever** — in any state, named or not | +| a tracker | a working copy of something the tracker already has | updates the tracker, then deletes the file | removes it once `state: closed` | **A successful push deletes `tmp/issues/.md`** (and `.comments.md`), on create and on `--update` alike. What is in the store is what has not left this machine; everything else is fetched again when it is needed. The rule, its safety conditions, and how the slug survives are `/tea:sync`'s to state. +**A closed issue is evicted from the store** by `issue_evict.py` — same trade, +one condition more: the work is done *and* it exists somewhere else. An +`origin: local` issue is never evicted, because there is nowhere to fetch it +back from. The store is a working set, not an archive; `pull.py ` fetches a +closed issue again whenever it is wanted. + The `id` never changes across that round trip, which is why `depends:` in other issues keeps working. That is the format's promise; the mechanism is not. diff --git a/skills/issue/scripts/issue.py b/skills/issue/scripts/issue.py index 491f5bd..b52e079 100644 --- a/skills/issue/scripts/issue.py +++ b/skills/issue/scripts/issue.py @@ -635,6 +635,35 @@ def all_ids(root): and "." not in f[:-3]) +def slug_files(root, id): + """Every file the store holds under one slug — the issue and its sidecars. + + `.md` is the issue. Anything named `.` beside it is a + companion another layer parked there (`.comments.md` is the one that + exists today). `all_ids` already refuses to read those as issues because a + slug has no dot in it; this is the same rule read the other way round. + + Which is how the domain can remove an issue *completely* without learning + what any of those companions are: it does not need to know that a comment + thread exists to know that a file named after this issue belongs to it and + goes when it goes. The issue's own file comes first — it is the headline of + any receipt printed from this list. + + A missing store is an empty list, not an error: nothing is there to remove. + """ + if not os.path.isdir(root): + return [] + own, sidecars = [], [] + for name in sorted(os.listdir(root)): + if not name.startswith("%s." % id): + continue + p = os.path.join(root, name) + if not os.path.isfile(p): + continue + (own if name == "%s.md" % id else sidecars).append(p) + return own + sidecars + + def load(root, id): with open(path_of(root, id)) as f: return Issue.from_text(f.read(), id=id) diff --git a/skills/issue/scripts/issue_evict.py b/skills/issue/scripts/issue_evict.py new file mode 100644 index 0000000..f858181 --- /dev/null +++ b/skills/issue/scripts/issue_evict.py @@ -0,0 +1,177 @@ +#!/usr/bin/env python3 +""" +issue_evict.py — closed issues leave the store. Offline. + + issue_evict.py every closed issue that is not origin: local + issue_evict.py old-thing … only these + issue_evict.py --dry-run print what would go; touch nothing + +The store is a working set, not an archive. A closed issue is not a unit of +work any more, and `pull.py` has kept new ones out of filter mode for a while — +but the files already on disk were nobody's job, so the only way to remove one +was `rm` past every script, followed by rebuilding `INDEX.md` by hand. This is +that job. + +WHAT IS EVICTED, and it is two conditions, both read off the file: + + state: closed the work is done + origin: the work is somewhere else too + +TWO CONDITIONS, AND THE SECOND ONE IS THE WHOLE SAFETY ARGUMENT. `origin: +local` means this file IS the issue — there is no other copy and deleting it +deletes the work. It is therefore never evicted, in any state, not even when +named explicitly on the command line: a closed local issue is reported and +kept. The only files that go are ones whose own metadata says the work can be +fetched back (`pull.py `), which is the same trade `push.py` makes when it +drops a file the tracker has just confirmed. + +That parallel is exact except for where the confirmation comes from. Push has +to ask Gitea, because it is Gitea that just changed. Eviction asks the file, +because `state:` and `origin:` are domain fields and the answer is already in +the store — which is why this command lives in the domain layer and needs no +network, no login, and no `tea`. See `skills/sync/scripts/evict.py` for the +variant that refreshes `state:` from the tracker first; it makes the deletion +decision by calling `run()` below, so there is exactly one implementation of +"what may be evicted" and it is this one. + +NOT A ONE-OFF MIGRATION. `pull.py ` fetches an issue in any state — a number +is an address, not a query — so a closed issue pulled after an eviction lands on +disk again. That is the tracker being asked a direct question, not a regression, +and the answer is to evict again when you are done with it. + +`.remote.json` is deliberately NOT pruned. It is the local number -> slug +ledger, its entries outlive the files they name (that is what makes `pull.py +` land on the same slug after a push deleted the file), and an evicted issue +is in exactly that state. `INDEX.md` is rebuilt, because it *is* a view of the +directory. +""" +import argparse +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +import issue # noqa: E402 +import issue_index # noqa: E402 + +CLOSED = "closed" + +# Why an issue was kept, in the receipt. `LOCAL_REASON` is the one that matters: +# it is printed whether or not the issue was named, because "this closed thing +# is still here" needs an answer every time. +LOCAL_REASON = "origin: %s — this file IS the issue" % issue.LOCAL + + +def classify(issues, ids=None): + """Split the store into (evict, protected, still_open). + + Pure — it reads the loaded issues and decides; nothing here touches disk. + + evict closed, and lives in a tracker too: safe to remove + protected closed, but `origin: local`: the only copy of the work + still_open not closed + + `ids` restricts the question to those issues; without it the whole store is + considered. A protected issue is returned as such even when it was named + explicitly — naming a file does not make deleting it safe. + """ + chosen = list(ids) if ids else sorted(issues) + evict, protected, still_open = [], [], [] + for id in chosen: + iss = issues[id] + if iss.state != CLOSED: + still_open.append(id) + elif iss.is_local: + protected.append(id) + else: + evict.append(id) + return evict, protected, still_open + + +def remove(root, id): + """Delete everything the store holds under one slug; return the paths. + + Deliberately dumb, and for the same reason `push.drop_local` is: it takes an + id, not a decision. Whether an issue may go is settled by `classify` before + this is reached, so the dangerous half of the operation has no branches in + it at all. There is exactly one call site. + """ + gone = [] + for p in issue.slug_files(root, id): + os.remove(p) + gone.append(p) + return gone + + +def run(root, issues, ids=None, dry_run=False, out=None): + """Classify, report, remove, rebuild the index. Returns (gone, kept). + + The one implementation of eviction, called both by `main` below and by the + sync layer's `evict.py` — which does nothing to this decision except hand + over issues whose `state:` it has just refreshed from the tracker. + + `gone` is {id: [paths]} and is empty on a dry run; `kept` is + [(id, why)] for everything considered and not removed. + """ + out = out or sys.stdout + evict, protected, still_open = classify(issues, ids) + + gone, kept = {}, [] + for id in evict: + paths = issue.slug_files(root, id) if dry_run else remove(root, id) + if not dry_run: + gone[id] = paths + out.write("%-11s %s\n" % ("would evict" if dry_run else "evicted", id)) + for p in paths: + out.write(" %s\n" % p) + for id in protected: + kept.append((id, LOCAL_REASON)) + out.write("%-11s %s closed, %s\n" % ("kept", id, LOCAL_REASON)) + # An open issue is the normal case and says nothing worth a line — unless + # the operator named it, in which case they are owed the reason. + for id in still_open: + kept.append((id, "state: %s" % issues[id].state)) + if ids: + out.write("%-11s %s state: %s\n" % ("kept", id, issues[id].state)) + + if dry_run: + out.write("%d issue(s) would be evicted, %d kept — nothing was touched\n" + % (len(evict), len(kept))) + return gone, kept + + out.write("%d issue(s) evicted, %d kept\n" % (len(gone), len(kept))) + # Only when something actually went: the index is a view of the directory, + # and rewriting it after a run that changed nothing is a write nobody asked + # for. + if gone: + path, n = issue_index.build(root) + out.write("index: %s — %d issue(s)\n" % (path, n)) + return gone, kept + + +def main(argv=None): + ap = argparse.ArgumentParser( + description="Evict closed issues from the local store (offline)") + ap.add_argument("ids", nargs="*", + help="issue ids (default: every closed issue in the store)") + ap.add_argument("--dry-run", action="store_true", + help="print what would be removed; touch nothing") + ap.add_argument("--out", default=issue.ISSUE_ROOT, + help="store root (default: /tmp/issues)") + args = ap.parse_args(argv) + + root = args.out + if not issue.store_exists(root): + sys.exit("issue_evict.py: store %s does not exist — nothing to evict" % root) + + issues = issue.load_all(root) + missing = [i for i in args.ids if i not in issues] + if missing: + sys.exit("issue_evict.py: no such issue(s) in the store: %s" + % ", ".join(missing)) + + run(root, issues, args.ids, args.dry_run) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/skills/sync/SKILL.md b/skills/sync/SKILL.md index 25110e1..2272772 100644 --- a/skills/sync/SKILL.md +++ b/skills/sync/SKILL.md @@ -40,6 +40,7 @@ the `tea-guard` hook reads. No pin → exit with a pointer to `/tea:auth`. | `remote.py [--state] [--label] [--milestone] [-q TEXT]` | discovery: one line per Gitea issue to stdout, writes nothing | | `pull.py ` or `pull.py --milestone M \| --label L \| -q TEXT` | Gitea → `tmp/issues/.md`, plus `.comments.md` when the thread is not empty | | `push.py [id…] [--update] [--dry-run]` | local → Gitea; validates first, **deletes the local file on success** and prints where it lives now | +| `evict.py [id…] [--dry-run]` | refresh `state:` from Gitea, then evict the issues it reports closed; `origin: local` is never asked about and never removed | | `comment.py --file F \| --body TEXT [--edit N]` | post or edit a comment, then refetch the thread | | `labels.py [--dry-run] [--fix]` | bootstrap the canonical `type/*` + `severity/*` set in a repo; exact names left alone, lookalikes reported, drift fixed only with `--fix` | | `map.py`, `_gitea.py` | the two layers the commands import — not commands | @@ -308,6 +309,57 @@ a git repo no `ref` is sent and a warning names the issues that went up without one. Reading the branch is the only thing these scripts ask git for — they never check out, create, or write anything. +## Evicting what the tracker says is closed + +```bash +python3 /scripts/evict.py --dry-run # ask, report, change nothing +python3 /scripts/evict.py # and remove them +python3 /scripts/evict.py old-thing # just this one +``` + +Eviction itself belongs to `/tea:issue` (`issue_evict.py`) and is offline: the +decision is `state: closed` plus an `origin:` that names a tracker, both read +off the file. This script adds one thing in front of it — a `state:` that is not +stale — and then calls that same decision. There is one implementation of "what +may be evicted" and it is in the domain. + +Why it exists: a local `state:` is only as fresh as the last pull, so an issue +closed in the web UI still reads `open` here and the offline command correctly +leaves it alone. The workaround was `pull.py 11 12 13 14 15` — which writes the +five closed files back to disk before anything can remove them. + +Order of operations, and it is the safety argument: + +1. every candidate's state is fetched — **all** of them, before anything is + removed; +2. each answer must be an object carrying the number that was asked about and a + state the domain recognizes (`evict.confirmed_state`, the counterpart of + `push.confirmed_number`); +3. only then does the eviction run. + +**A failed call evicts nothing** — not even the candidates whose answers had +already arrived, and no refreshed `state:` is written back either. Stricter than +push, which deletes as it goes, and free: evictions have no order between them, +so there is no reason to start before every answer is in. + +- A **candidate** is an issue carrying a `gitea:` handle. `origin: local` has + none, is never asked about, and is never removed. An `origin: gitea` issue + whose handle is missing or unparseable cannot be verified — it is reported on + stderr and kept. +- No `--repo`: the repo comes from each issue's own handle, so a store holding + issues from two repos is checked against both. +- One GET per candidate. The store is a working set that push keeps small, and a + wrong answer here deletes a file — so each issue is asked about by its own + address rather than inferred from a list a `--limit` could have truncated. +- A state that disagrees with the file is written back, so the store stops lying + about the issues that stay too. `--dry-run` makes no writes at all. +- `.remote.json` is not pruned; see [How the slug comes + back](#how-the-slug-comes-back) — an evicted issue is exactly as findable as a + pushed one. +- **`pull.py ` still fetches a closed issue.** A number is an address, not a + query. A closed issue pulled after an eviction is back on disk, and that is + the tracker answering the question it was asked, not a regression. + ## What crosses the boundary, and what does not | domain | Gitea | note | diff --git a/skills/sync/scripts/evict.py b/skills/sync/scripts/evict.py new file mode 100644 index 0000000..46619b7 --- /dev/null +++ b/skills/sync/scripts/evict.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +""" +evict.py — ask Gitea which stored issues are closed, then evict those. + + evict.py check every synced issue in the store, evict the + ones Gitea says are closed + evict.py old-thing … only these + evict.py --dry-run ask, report, change nothing + +The offline command is `/tea:issue`'s `issue_evict.py`, and it is the one that +decides and deletes — this script adds exactly one thing in front of it: a +`state:` that is not stale. A local `state:` is only as fresh as the last pull, +so an issue closed in the web UI an hour ago still reads `open` here and the +offline command will (correctly) leave it alone. That is the gap this closes, +and it is the observed workflow: before this existed the operator had to +`pull.py 11 12 13 14 15` first, which re-wrote the five closed files onto disk +before anything could remove them. + +Order of operations, and it is the whole safety argument: + + 1. every candidate's state is fetched — ALL of them, before anything is + removed; + 2. each answer must be an object carrying the number we asked about and a + state from the domain's own vocabulary (`confirmed_state`); + 3. only then is the eviction run, by handing the refreshed issues to + `issue_evict.run` — the same decision, the same deletion, the same + protection of `origin: local`, in one place. + +A `tea` that will not run, a non-2xx, an answer for another issue, a state +nobody recognizes: the run stops at step 2 and NOTHING is deleted, not even the +issues whose answers had already arrived. That is stricter than `push.py`, which +deletes as it goes, and it costs nothing here — there is no ordering constraint +between evictions, so there is no reason to start before every answer is in. + +A candidate is an issue carrying a `gitea:` handle. `origin: local` work has +none, is never asked about, and is never evicted — it is not in the tracker to +be closed. An `origin: gitea` issue whose handle is missing or unparseable +cannot be verified, so it is reported and kept rather than guessed at. + +Cost: one GET per candidate. The store is a working set that push keeps small, +and a wrong answer here deletes a file, so each issue is asked about by its own +address rather than inferred from a list that a `--limit` could have truncated. + +Login: the operator's pin from .claude/settings.local.json (see /tea:auth). +""" +import argparse +import os +import sys + +_HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path[:0] = [_HERE, os.path.normpath(os.path.join(_HERE, "..", "..", "issue", "scripts"))] + +import _gitea # noqa: E402 +import issue # noqa: E402 +import issue_evict # noqa: E402 +import map as gmap # noqa: E402 + + +def candidates(issues, ids=None): + """(checkable, unverifiable) — which issues the tracker can be asked about. + + checkable is [(id, repo, number)] read off the `gitea:` handle, so an issue + that lives in another repo is asked about there. unverifiable is + [(id, why)]: it names a tracker but carries no handle to reach it by, which + is a file to report, never one to delete on a guess. + + An `origin: local` issue is in neither list. It has no handle because it has + never left this machine, and asking Gitea about it is not a question that + has an answer. + """ + checkable, unverifiable = [], [] + for id in (list(ids) if ids else sorted(issues)): + iss = issues[id] + if iss.is_local: + continue + repo, number = gmap.parse_remote_key(iss.extra.get("gitea", "")) + if not repo or not number: + unverifiable.append((id, "origin: %s but no usable `gitea:` handle" + % iss.origin)) + continue + checkable.append((id, repo, number)) + return checkable, unverifiable + + +def confirmed_state(got, number): + """The state Gitea confirmed for `number`, or None — the deletion gate. + + The counterpart of `push.confirmed_number`, and written the same way: boring, + and saying no by default, because everything downstream of a `str` return + here may delete a file. An answer counts only when it is a dict, carries the + very number we asked about, and names a state the domain recognizes. + + `bool` is rejected explicitly: `True` is an `int` in Python, and an answer + about issue `true` is not an answer about issue 42. + + What it does not have to catch, because it never gets here: a non-2xx or a + `tea` that would not run at all — `_gitea.api` exits on both. + """ + if not isinstance(got, dict): + return None + n = got.get("number") + if isinstance(n, bool) or not isinstance(n, int) or n != number: + return None + state = got.get("state") + return state if state in issue.STATES else None + + +def main(argv=None): + ap = argparse.ArgumentParser( + description="Evict issues Gitea reports as closed from the local store") + ap.add_argument("ids", nargs="*", + help="issue ids (default: every synced issue in the store)") + ap.add_argument("--dry-run", action="store_true", + help="ask the tracker and report; write and delete nothing") + ap.add_argument("--out", default=issue.ISSUE_ROOT, + help="store root (default: /tmp/issues)") + args = ap.parse_args(argv) + + root = args.out + if not issue.store_exists(root): + _gitea.die("store %s does not exist — nothing to evict" % root) + issues = issue.load_all(root) + missing = [i for i in args.ids if i not in issues] + if missing: + _gitea.die("no such issue(s) in the store: %s" % ", ".join(missing)) + + checkable, unverifiable = candidates(issues, args.ids) + for id, why in unverifiable: + _gitea.warn("%s: %s — kept, and not asked about" % (id, why)) + if not checkable: + print("nothing to check: no issue in the store carries a `gitea:` handle") + return 0 + + login = _gitea.require_login() + + # ---- every answer first, deletions after ----------------------------- + fresh = {} + for id, repo, number in checkable: + got = _gitea.api(login, "%s/issues/%d" % (_gitea.repo_base(repo), number)) + state = confirmed_state(got, number) + if state is None: + _gitea.die("%s: the tracker's answer for %s#%d does not confirm a state " + "(%.200r). Nothing was evicted." + % (id, repo, number, got)) + fresh[id] = state + + # The store stops lying even about the issues that stay: an answer already + # paid for is written back when it disagrees with the file. This is the only + # write this script makes, and a dry run makes none. + for id, state in sorted(fresh.items()): + was = issues[id].state + if was == state: + continue + print("state %s %s -> %s" % (id, was, state)) + issues[id].state = state + if not args.dry_run: + issue.save(root, issues[id]) + + issue_evict.run(root, issues, [id for id, _, _ in checkable], args.dry_run) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_evict_closed.py b/tests/test_evict_closed.py new file mode 100644 index 0000000..b643037 --- /dev/null +++ b/tests/test_evict_closed.py @@ -0,0 +1,570 @@ +#!/usr/bin/env python3 +""" +Closed issues leave the store, and nothing else does. + +Two halves, and the second one is the one that matters: + +1. **It evicts.** A closed issue whose `origin:` names a tracker is removed from + `tmp/issues/` — the issue file and every sidecar under its slug — by one + command, and `INDEX.md` is rebuilt so the directory and its table agree. + `skills/sync/scripts/evict.py` does the same after refreshing `state:` from + Gitea, so an issue closed in the web UI goes without a pull first. + +2. **It evicts nothing else, ever.** `origin: local` is the only copy of the + work there is: it stays in every state, including when it is closed and + including when it is named on the command line. An open issue stays. A dry + run stays. And a tracker call that fails leaves the whole store on disk — + every candidate, not just the ones whose answers had not arrived yet. + +A bug in the second half destroys work, so each path is asserted separately and +the assertion is always the same — `os.path.isfile`. + +Nothing here touches a network (the sync half stubs `_gitea.api`, and one test +stubs `_gitea.subprocess` so a non-zero `tea` is proved end to end) and nothing +here touches the developer's store: every test builds its own under +`tempfile.TemporaryDirectory()`. +""" +import contextlib +import io +import os +import shutil +import sys +import tempfile +import types +import unittest +from unittest import mock + +_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +for _p in (os.path.join(_ROOT, "skills", "sync", "scripts"), + os.path.join(_ROOT, "skills", "issue", "scripts")): + if _p not in sys.path: + sys.path.insert(0, _p) + +import _gitea # noqa: E402 +import evict # noqa: E402 +import issue # noqa: E402 +import issue_evict # noqa: E402 +import map as gmap # noqa: E402 + +REAL_API = _gitea.api + +REPO = "claude-skills/tea" + +BODY = """## Summary +Прозаическое описание задачи. + +## Spec +skills/issue/references/format.md + +## Acceptance criteria +- [x] сделано +""" + + +class StoreTestCase(unittest.TestCase): + """A temp store, and fixtures for the three kinds of file that live in it.""" + + def setUp(self): + self.root = tempfile.mkdtemp(prefix="tea-evict-") + self.addCleanup(shutil.rmtree, self.root, True) + self.numbers = {} + + # -- fixtures ---------------------------------------------------------- + + def local(self, id, state="open"): + """An issue that exists nowhere but here.""" + return self._write(id, state=state, origin=issue.LOCAL) + + def synced(self, id, state="open", number=None): + """A working copy of something the tracker already has.""" + n = number if number is not None else 100 + len(self.numbers) + self.numbers[id] = n + return self._write(id, state=state, origin=gmap.ORIGIN, + extra={"gitea": gmap.remote_key(REPO, n), + "url": "https://git.example/%s/issues/%d" % (REPO, n), + "synced": "2026-08-10T00:00:00Z"}) + + def _write(self, id, state, origin, extra=None): + iss = issue.Issue(id=id, title=id.replace("-", " ").capitalize(), + body=BODY, labels=["type/task"], state=state, + origin=origin, extra=dict(extra or {})) + issue.save(self.root, iss) + return iss + + def comments(self, id): + p = _gitea.comments_path(self.root, id) + with open(p, "w") as f: + f.write("## comment 1 — someone — 2026-08-10\n\nтекст\n") + return p + + # -- runners ----------------------------------------------------------- + + def run_evict(self, *argv): + return self._run(issue_evict, "issue_evict.py", argv) + + def run_sync_evict(self, *argv): + return self._run(evict, "evict.py", argv) + + def _run(self, mod, name, argv): + self.out, self.err = io.StringIO(), io.StringIO() + args = [name, "--out", self.root] + list(argv) + with mock.patch.object(sys, "argv", args), \ + contextlib.redirect_stdout(self.out), \ + contextlib.redirect_stderr(self.err): + mod.main() + return self.out.getvalue(), self.err.getvalue() + + # -- assertions -------------------------------------------------------- + + def assertOnDisk(self, id, why=""): + self.assertTrue(os.path.isfile(issue.path_of(self.root, id)), + "%s.md was deleted%s" % (id, why and " — " + why)) + + def assertGone(self, id): + self.assertFalse(os.path.isfile(issue.path_of(self.root, id)), + "%s.md is still on disk" % id) + + def index(self): + with open(os.path.join(self.root, "INDEX.md")) as f: + return f.read() + + +# -------------------------------------------------------------------------- +# the domain: what belongs to a slug +# -------------------------------------------------------------------------- + +class SlugFilesTest(StoreTestCase): + """`issue.slug_files` — how the domain removes an issue completely without + knowing what a comment thread is.""" + + def test_the_issue_file_comes_first(self): + self.synced("a-thing") + p = self.comments("a-thing") + self.assertEqual(issue.slug_files(self.root, "a-thing"), + [issue.path_of(self.root, "a-thing"), p]) + + def test_an_issue_with_no_sidecars_is_one_file(self): + self.synced("a-thing") + self.assertEqual(issue.slug_files(self.root, "a-thing"), + [issue.path_of(self.root, "a-thing")]) + + def test_a_longer_slug_is_not_a_sidecar(self): + """`a-thing-2` is another issue, not a companion of `a-thing`.""" + self.synced("a-thing") + self.synced("a-thing-2") + self.assertEqual(issue.slug_files(self.root, "a-thing"), + [issue.path_of(self.root, "a-thing")]) + + def test_a_missing_store_is_empty_not_an_error(self): + self.assertEqual(issue.slug_files(os.path.join(self.root, "nope"), "x"), []) + + +# -------------------------------------------------------------------------- +# the domain: it evicts +# -------------------------------------------------------------------------- + +class EvictsClosedTest(StoreTestCase): + + def test_a_closed_synced_issue_goes(self): + self.synced("old-thing", state="closed") + self.run_evict() + self.assertGone("old-thing") + + def test_the_comment_thread_goes_with_it(self): + self.synced("old-thing", state="closed") + p = self.comments("old-thing") + self.run_evict() + self.assertFalse(os.path.isfile(p), "the thread outlived the issue") + + def test_the_store_of_open_and_closed_keeps_exactly_the_open_and_the_local(self): + """The acceptance criterion, whole: a store of both kinds, one run, and + what is left is the open issues and the local ones.""" + self.synced("open-synced") + self.synced("closed-synced", state="closed") + self.local("open-local") + self.local("closed-local", state="closed") + + self.run_evict() + + self.assertEqual(issue.all_ids(self.root), + ["closed-local", "open-local", "open-synced"]) + + def test_the_output_names_every_file_removed(self): + self.synced("old-thing", state="closed") + p = self.comments("old-thing") + out, _ = self.run_evict() + self.assertIn("evicted", out) + self.assertIn(issue.path_of(self.root, "old-thing"), out) + self.assertIn(p, out) + + def test_the_index_is_rebuilt_to_match_the_directory(self): + """`INDEX.md` and the directory agree afterwards — nothing to fix up.""" + self.synced("old-thing", state="closed") + self.synced("live-thing") + self.run_evict() + index = self.index() + self.assertIn("live-thing", index) + self.assertNotIn("old-thing", index) + + def test_only_the_named_issue_is_evicted(self): + self.synced("first-old", state="closed") + self.synced("second-old", state="closed") + self.run_evict("first-old") + self.assertGone("first-old") + self.assertOnDisk("second-old", "it was not named") + + def test_the_ledger_is_not_pruned(self): + """`.remote.json` is the number -> slug ledger, not an index over the + files: an evicted issue is exactly as findable as a pushed one.""" + self.synced("old-thing", state="closed") + key = gmap.remote_key(REPO, self.numbers["old-thing"]) + _gitea.save_map(self.root, {key: "old-thing"}) + self.run_evict() + self.assertEqual(_gitea.load_map(self.root), {key: "old-thing"}) + + +class ClassifyTest(unittest.TestCase): + """The decision itself, pure. Everything below it deletes a file.""" + + def issues(self, **kinds): + return {id: issue.Issue(id=id, state=state, origin=origin) + for id, (state, origin) in kinds.items()} + + def test_closed_and_synced_is_evicted(self): + got = issue_evict.classify(self.issues(a=("closed", "gitea"))) + self.assertEqual(got, (["a"], [], [])) + + def test_closed_and_local_is_protected(self): + got = issue_evict.classify(self.issues(a=("closed", issue.LOCAL))) + self.assertEqual(got, ([], ["a"], [])) + + def test_open_is_left_alone_whatever_its_origin(self): + got = issue_evict.classify(self.issues(a=("open", "gitea"), + b=("open", issue.LOCAL))) + self.assertEqual(got, ([], [], ["a", "b"])) + + def test_naming_a_local_issue_does_not_make_it_evictable(self): + got = issue_evict.classify(self.issues(a=("closed", issue.LOCAL)), ["a"]) + self.assertEqual(got, ([], ["a"], [])) + + def test_ids_restrict_the_question(self): + got = issue_evict.classify(self.issues(a=("closed", "gitea"), + b=("closed", "gitea")), ["b"]) + self.assertEqual(got, (["b"], [], [])) + + +# -------------------------------------------------------------------------- +# the domain: it evicts nothing else +# -------------------------------------------------------------------------- + +class LocalIsNeverEvictedTest(StoreTestCase): + """The criterion that matters most: `origin: local` IS the work.""" + + def test_a_closed_local_issue_stays(self): + self.local("closed-local", state="closed") + self.run_evict() + self.assertOnDisk("closed-local", "origin: local is the only copy") + + def test_a_closed_local_issue_named_explicitly_still_stays(self): + self.local("closed-local", state="closed") + out, _ = self.run_evict("closed-local") + self.assertOnDisk("closed-local", "naming it does not make deleting it safe") + self.assertIn("kept", out) + + def test_the_receipt_says_why_it_was_kept(self): + self.local("closed-local", state="closed") + out, _ = self.run_evict() + self.assertIn("origin: local", out) + self.assertIn("this file IS the issue", out) + + def test_its_sidecars_stay_too(self): + self.local("closed-local", state="closed") + p = self.comments("closed-local") + self.run_evict() + self.assertTrue(os.path.isfile(p)) + + +class DryRunTouchesNothingTest(StoreTestCase): + + def test_nothing_is_deleted(self): + self.synced("old-thing", state="closed") + p = self.comments("old-thing") + self.run_evict("--dry-run") + self.assertOnDisk("old-thing", "--dry-run must not delete") + self.assertTrue(os.path.isfile(p)) + + def test_it_prints_what_would_go(self): + self.synced("old-thing", state="closed") + p = self.comments("old-thing") + out, _ = self.run_evict("--dry-run") + self.assertIn("would evict", out) + self.assertIn(issue.path_of(self.root, "old-thing"), out) + self.assertIn(p, out) + self.assertIn("nothing was touched", out) + + def test_the_index_is_not_written(self): + """`INDEX.md` is a write like any other — a dry run makes none.""" + self.synced("old-thing", state="closed") + self.run_evict("--dry-run") + self.assertFalse(os.path.isfile(os.path.join(self.root, "INDEX.md"))) + + +class NoOpRunsWriteNothingTest(StoreTestCase): + + def test_a_store_with_nothing_to_evict_is_not_rewritten(self): + self.synced("live-thing") + out, _ = self.run_evict() + self.assertIn("0 issue(s) evicted", out) + self.assertFalse(os.path.isfile(os.path.join(self.root, "INDEX.md"))) + + def test_an_unknown_id_stops_the_run(self): + self.synced("old-thing", state="closed") + with self.assertRaises(SystemExit): + self.run_evict("no-such-thing") + self.assertOnDisk("old-thing", "the run stopped before anything went") + + def test_a_missing_store_is_an_error_and_not_a_directory_to_create(self): + missing = os.path.join(self.root, "nope") + self.out, self.err = io.StringIO(), io.StringIO() + argv = ["issue_evict.py", "--out", missing] + with mock.patch.object(sys, "argv", argv), \ + contextlib.redirect_stdout(self.out), \ + contextlib.redirect_stderr(self.err), \ + self.assertRaises(SystemExit): + issue_evict.main() + self.assertFalse(os.path.isdir(missing)) + + +# -------------------------------------------------------------------------- +# the bridge: the state comes from the tracker +# -------------------------------------------------------------------------- + +class FakeTracker(object): + """`tea api` answered from memory. GET on an issue, and nothing else.""" + + def __init__(self): + self.calls = [] + self.states = {} # number -> "open" | "closed" + self.answer_override = {} # number -> whatever it should answer instead + self.raise_on = None # number -> exception to raise instead + + def api(self, login, endpoint, method="GET", payload=None, payload_name=None, + out_root=None, allow_fail=False): + self.calls.append((method, endpoint)) + number = int(endpoint.rstrip("/").rsplit("/", 1)[1]) + if self.raise_on == number: + raise OSError("tea: command not found") + if number in self.answer_override: + return self.answer_override[number] + return {"number": number, "state": self.states.get(number, "open"), + "title": "Whatever", "body": "текст"} + + +class SyncEvictTestCase(StoreTestCase): + + def setUp(self): + StoreTestCase.setUp(self) + self.fake = FakeTracker() + for p in (mock.patch.object(_gitea, "api", self.fake.api), + mock.patch.object(_gitea, "require_login", lambda: "test-login")): + p.start() + self.addCleanup(p.stop) + + def close_in_gitea(self, id): + self.fake.states[self.numbers[id]] = "closed" + + def state_on_disk(self, id): + return issue.load(self.root, id).state + + +class TrackerStateWinsTest(SyncEvictTestCase): + + def test_an_issue_closed_upstream_is_evicted_without_a_pull_first(self): + """The observed workflow, in one command: the file still says `open`.""" + self.synced("old-thing", state="open") + self.close_in_gitea("old-thing") + self.run_sync_evict() + self.assertGone("old-thing") + + def test_an_issue_still_open_upstream_stays(self): + self.synced("live-thing", state="open") + self.run_sync_evict() + self.assertOnDisk("live-thing", "Gitea says it is open") + + def test_a_stale_closed_file_is_corrected_and_kept(self): + """Reopened in the web UI: the local `state:` stops lying, and the file + is not evicted on the strength of what it used to say.""" + self.synced("back-thing", state="closed") + self.run_sync_evict() + self.assertOnDisk("back-thing", "Gitea says it is open again") + self.assertEqual(self.state_on_disk("back-thing"), "open") + + def test_a_local_issue_is_never_asked_about(self): + self.local("closed-local", state="closed") + out, _ = self.run_sync_evict() + self.assertEqual(self.fake.calls, []) + self.assertOnDisk("closed-local") + + def test_an_issue_with_no_handle_is_reported_and_kept(self): + """`origin: gitea` and nothing to reach it by: a guess would delete a + file nobody can get back.""" + issue.save(self.root, issue.Issue(id="orphan-thing", title="Orphan thing", + body=BODY, labels=["type/task"], + state="closed", origin=gmap.ORIGIN)) + _, err = self.run_sync_evict() + self.assertIn("orphan-thing", err) + self.assertOnDisk("orphan-thing", "it could not be verified") + + def test_the_index_matches_the_directory_afterwards(self): + self.synced("old-thing", state="open") + self.synced("live-thing", state="open") + self.close_in_gitea("old-thing") + self.run_sync_evict() + self.assertNotIn("old-thing", self.index()) + self.assertIn("live-thing", self.index()) + + def test_dry_run_asks_but_neither_writes_nor_deletes(self): + self.synced("old-thing", state="open") + self.close_in_gitea("old-thing") + out, _ = self.run_sync_evict("--dry-run") + self.assertTrue(self.fake.calls, "it should still have asked") + self.assertOnDisk("old-thing", "--dry-run must not delete") + self.assertEqual(self.state_on_disk("old-thing"), "open", + "--dry-run must not write the refreshed state either") + self.assertIn("would evict", out) + + +class SurvivesEveryTrackerFailureTest(SyncEvictTestCase): + """A failed call evicts nothing — including the candidates whose answers had + already arrived.""" + + def two_closed(self): + self.synced("aaa-thing", state="closed", number=11) + self.synced("zzz-thing", state="closed", number=12) + self.close_in_gitea("aaa-thing") + self.close_in_gitea("zzz-thing") + + def test_a_transport_exception_evicts_nothing(self): + self.two_closed() + self.fake.raise_on = 12 + with self.assertRaises(OSError): + self.run_sync_evict() + self.assertOnDisk("aaa-thing", "its answer arrived, but the run failed") + self.assertOnDisk("zzz-thing") + + def test_a_non_2xx_answer_evicts_nothing(self): + """The real `_gitea.api` against a `tea` that exits 1 — the path a 422 + or a 500 actually takes, and it ends in `die()`.""" + self.two_closed() + + def fake_run(cmd, capture_output=False, text=False): + return types.SimpleNamespace(returncode=1, stdout="", + stderr="500 Internal Server Error") + + with mock.patch.object(_gitea, "api", REAL_API), \ + mock.patch.object(_gitea, "subprocess", + types.SimpleNamespace(run=fake_run)), \ + self.assertRaises(SystemExit): + self.run_sync_evict() + + self.assertOnDisk("aaa-thing", "tea exited non-zero") + self.assertOnDisk("zzz-thing", "tea exited non-zero") + + def test_an_answer_for_another_issue_evicts_nothing(self): + self.two_closed() + self.fake.answer_override[12] = {"number": 999, "state": "closed"} + with self.assertRaises(SystemExit): + self.run_sync_evict() + self.assertOnDisk("aaa-thing") + self.assertOnDisk("zzz-thing", "the tracker answered for a different issue") + + def test_an_answer_without_a_state_evicts_nothing(self): + self.two_closed() + self.fake.answer_override[12] = {"number": 12} + with self.assertRaises(SystemExit): + self.run_sync_evict() + self.assertOnDisk("aaa-thing") + self.assertOnDisk("zzz-thing") + + def test_an_empty_answer_evicts_nothing(self): + """`tea` exited 0 and printed nothing — api returns None.""" + self.two_closed() + self.fake.answer_override[12] = None + with self.assertRaises(SystemExit): + self.run_sync_evict() + self.assertOnDisk("aaa-thing") + self.assertOnDisk("zzz-thing") + + def test_the_error_says_nothing_was_evicted(self): + self.two_closed() + self.fake.answer_override[12] = {"ok": True} + with self.assertRaises(SystemExit): + self.run_sync_evict() + self.assertIn("Nothing was evicted", self.err.getvalue()) + + def test_no_state_is_written_back_before_the_failure_either(self): + """The write-back happens after every answer is in, so a run that dies + leaves the files exactly as it found them.""" + self.synced("aaa-thing", state="closed", number=11) + self.synced("zzz-thing", state="closed", number=12) + self.fake.states[11] = "open" # would be corrected on a good run + self.fake.answer_override[12] = {"nope": True} + with self.assertRaises(SystemExit): + self.run_sync_evict() + self.assertEqual(self.state_on_disk("aaa-thing"), "closed") + + +class ConfirmedStateTest(unittest.TestCase): + """The gate itself, in the shape of `push.confirmed_number`.""" + + def test_a_matching_answer_is_confirmed(self): + self.assertEqual(evict.confirmed_state({"number": 42, "state": "closed"}, 42), + "closed") + self.assertEqual(evict.confirmed_state({"number": 42, "state": "open"}, 42), + "open") + + def test_another_issue_is_not(self): + self.assertIsNone(evict.confirmed_state({"number": 43, "state": "closed"}, 42)) + + def test_none_is_not(self): + self.assertIsNone(evict.confirmed_state(None, 42)) + + def test_a_list_is_not(self): + self.assertIsNone(evict.confirmed_state([{"number": 42, "state": "closed"}], 42)) + + def test_a_missing_state_is_not(self): + self.assertIsNone(evict.confirmed_state({"number": 42}, 42)) + + def test_an_unknown_state_is_not(self): + self.assertIsNone(evict.confirmed_state({"number": 42, "state": "merged"}, 42)) + + def test_a_string_number_is_not(self): + self.assertIsNone(evict.confirmed_state({"number": "42", "state": "closed"}, 42)) + + def test_true_is_not_a_number(self): + self.assertIsNone(evict.confirmed_state({"number": True, "state": "closed"}, 1)) + + +class CandidatesTest(StoreTestCase): + """Who the tracker is asked about at all.""" + + def test_a_synced_issue_is_asked_about_in_its_own_repo(self): + self.synced("a-thing", number=7) + checkable, unverifiable = evict.candidates(issue.load_all(self.root)) + self.assertEqual(checkable, [("a-thing", REPO, 7)]) + self.assertEqual(unverifiable, []) + + def test_a_local_issue_is_in_neither_list(self): + self.local("local-thing", state="closed") + self.assertEqual(evict.candidates(issue.load_all(self.root)), ([], [])) + + def test_a_handle_that_cannot_be_parsed_is_unverifiable(self): + issue.save(self.root, issue.Issue(id="bad-thing", origin=gmap.ORIGIN, + extra={"gitea": "not-a-key"})) + checkable, unverifiable = evict.candidates(issue.load_all(self.root)) + self.assertEqual(checkable, []) + self.assertEqual([id for id, _ in unverifiable], ["bad-thing"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_store_path.py b/tests/test_store_path.py index 736d214..0d9fabb 100644 --- a/tests/test_store_path.py +++ b/tests/test_store_path.py @@ -397,9 +397,10 @@ class TestSyncLayerAgrees(unittest.TestCase): """Both layers agree by construction, not by coincidence: no script spells the default out for itself.""" for layer, names in (("issue", ("issue_new.py", "issue_check.py", - "issue_tree.py", "issue_index.py")), + "issue_tree.py", "issue_index.py", + "issue_evict.py")), ("sync", ("pull.py", "push.py", "remote.py", - "comment.py"))): + "comment.py", "evict.py"))): for name in names: with open(os.path.join(REPO, "skills", layer, "scripts", name)) as f: src = f.read()