fix(desktop): stop hiding repos re-announced after a deletion - #3782
Open
daxdax89 wants to merge 1 commit into
Open
fix(desktop): stop hiding repos re-announced after a deletion#3782daxdax89 wants to merge 1 commit into
daxdax89 wants to merge 1 commit into
Conversation
`isDeletedByA` matched any same-author kind:5 tombstone carrying the project's `30617:<owner>:<dtag>` coordinate, with no timestamp comparison. Repo announcements are addressable, so the coordinate outlives any single event at it: deleting a repo and announcing the same dtag again is a legitimate recovery path. NIP-09 scopes an `a`-tag deletion to versions "up to the created_at timestamp of the deletion request event", so a tombstone must not hide an announcement published after it. The relay already gets this right, which made the failure confusing: the newer announcement was stored, served over REQ, and clone/push worked, while Desktop's Repositories list never rendered the card again for anyone. Bound the match to `event.created_at >= project.createdAt` and move the predicate (with `projectCoordinate`) into `lib/projectDeletions.ts` so it is unit-testable, which also keeps hooks.ts under the 1000-line guard. Fixes block#3760 Signed-off-by: DaX <daxdax89@gmail.com>
Contributor
|
good catch on re-announced repos. can you add a test where created_at equals the deletion time so the boundary is pinned? |
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.
Fixes #3760.
Problem
isDeletedByAhid a project card whenever any same-author kind:5 tombstone carried its30617:<owner>:<dtag>coordinate, with no timestamp comparison:Repo announcements are addressable, so the coordinate outlives any single event at it. Deleting a repo and announcing the same dtag again is a legitimate recovery path, and NIP-09 scopes an
a-tag deletion to versions "up to thecreated_attimestamp of the deletion request event" — a tombstone must not hide an announcement published after it.The relay already gets this right, which is what made the failure confusing to diagnose: the newer announcement is stored, served over REQ, and clone/push works, while Desktop's Repositories list never renders the card again for anyone. Once an owner had ever deleted a project (including via Desktop's own Delete project button), no re-announcement at that coordinate could come back.
Fix
Bound the match to
event.created_at >= project.createdAt, keeping the existing author check. The comparison is inclusive, matching NIP-09's "up to thecreated_attimestamp".The predicate and
projectCoordinatemove fromhooks.tsintolib/projectDeletions.ts. Two reasons: the predicate was unexported and therefore untestable, andhooks.tswas at 997 of the 1000 linesdesktop/scripts/check-file-sizes.mjsallows, so the fix could not have been added in place.hooks.tsis now 986 lines and both call sites (fetchProjects,fetchProject) are unchanged apart from the import.I kept the
isDeletedByAname so it still matches the issue.Tests
lib/projectDeletions.test.mjs— 8 cases covering the regression (tombstone older than the announcement no longer hides it), a newer tombstone still hiding it, and the inclusive same-timestamp boundary. The author check, case-insensitive pubkey match, and coordinate matching already worked and are pinned so a later refactor cannot quietly drop them.Verified the suite catches the bug: with the
created_atcomparison removed, exactly one test fails — "a tombstone older than the announcement does not hide it".Checks
just desktop-check,just desktop-typecheck,just desktop-test(3827 pass), andjust desktop-buildall pass locally.Out of scope
The issue's second half is deliberately not addressed here, since it needs the managed-agent ownership relationship on the client and is a larger change:
isDeletedByAonly honors deletions whose pubkey equals the announcement author, so a relay-accepted owner-of-agent deletion (is_agent_ownerinvalidate_standard_deletion_event) still leaves the card rendered.canDeleteinProjectsView.tsxgates on strict key equality viaisProjectOwnedByCurrentUser, so a user cannot delete their own managed agent's repos from Desktop even though the relay would authorize it.Happy to follow up on those separately if useful.
Related PRs
Searched open PRs and issues for duplicates; found none touching
isDeletedByAor repo-deletion filtering.