feat: record a 48h history of removed torrents with the reason why - #32
Merged
Merged
Conversation
Torrents can disappear from seedstrem through four independent paths, and only one of them is seed-time related, so after the fact there was no way to tell which component removed one or why. Un-adopting is the most confusing of the four: it drops the store row without touching the download client, so a torrent vanishes from seedstrem while it is still seeding in Deluge — easily misread as a seed-time bug. Record every completed removal for 48 hours together with the evidence behind the decision (observed seeding time against the effective limit, ratio against the target, progress against the threshold), and surface it on a new History page. - store: torrent_deletions table, RecordDeletion, Deletions. No foreign key to torrents: the row it describes is being deleted, so a cascade would destroy the evidence. Retention is enforced on read (correct on an idle instance) and pruned on write (bounds the table). - torrents: Service.Remove takes a store.DeletionEvent, making it a compile error to add a removal path without stating a reason. Callers in cleanup, stream and admin pass the evidence only they hold. - adopt: unadopt bypasses Service, so it records its own removal with files_deleted=false. - admin: GET /api/deletions, authenticated, durations in seconds. - web: History page and sidebar entry, with the per-reason wording kept in a pure, unit-tested module. vitest is added as a devDependency: package.json already declared "test": "vitest run" but it was never installed, so the project had no runnable JS tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Torrents disappear from seedstrem and there is no way, after the fact, to tell which code path removed one or why. Four independent paths delete a torrent, and only one is seed-time related:
Un-adopt is the sneaky one: it deletes the row without touching the download client, so a torrent "disappears from seedstrem" while still seeding in Deluge. That is easily misread as a seed-time bug.
Investigation along the way confirmed seed-time config is already applied live —
main.gohands cleanup a closure callingcm.Get(), andSweepre-reads it every pass, so raising or lowering the seed time takes effect for all current torrents on the next sweep. No per-torrent snapshot exists anywhere. That ruled out the obvious suspect and motivated this audit log.What changed
torrent_deletionstable (migration0005),RecordDeletion,Deletions. Deliberately no foreign key totorrents— the row it describes is being deleted, so a cascade would destroy the evidence. Retention is enforced on read (stays correct on an idle instance where no write has triggered a prune) and pruned on write (bounds the table).Service.Removenow takes astore.DeletionEvent. The signature change makes it a compile error to add a removal path without stating a reason. Identity andfiles_deletedare filled in centrally; callers pass only the evidence they uniquely hold.unadoptbypassesService, so it records its own removal withfiles_deleted=false.GET /api/deletions, authenticated, newest first, durations in seconds to match the torrents listing.The evidence columns are the point: a row reading
seed_time · seeded 49h of 48h · indexer=Xeither confirms or exonerates the seed-time path immediately.files_deleteddistinguishes "row gone, data intact" from "data wiped" — currently unrecoverable after the fact.Reviewer notes
Two decisions worth a second opinion:
vitestadded as a devDependency.package.jsonalready declared"test": "vitest run"but it was never installed, so the project had no runnable JS tests. This completes that existing intent, but it is a dependency change.progress_limitis its own column. I first reusedratio_limitto carry the abandoned path's threshold; it read as nonsense in the UI, so it got a dedicated field.Deliberately out of scope: failed removal attempts are not recorded (only completed ones), retention is a constant rather than config, and there is no filtering, pagination, or export. An immediate cleanup sweep on config save was discussed and parked.
Design spec is committed alongside the code at
docs/superpowers/specs/2026-08-19-deletion-history-design.md.Test plan
All verified locally:
go test ./...— every package okgo test -raceon store, cleanup, adopt, admin — okgo vet ./...— cleannpm test— 8 passednpm run build(includestsc -b) — okNew tests cover the store round-trip, that a record survives its torrent's deletion, the retention boundary (47h visible / 49h hidden), prune-on-write, ordering, one test per capture path including
unadopted(which had no coverage at all before), and the endpoint's shape, auth, and empty-array response.One process note: the abandoned-path test passed on first run because that call site was already written, so rather than let a vacuous test stand I temporarily broke the implementation and confirmed it failed with
progress limit = 0, want 0.05before restoring it. The other paths followed proper red-green.Not yet exercised against a real Deluge instance with an existing database; the migration applies through the standard pending-migration loop.