Repo delete leaves userspace references (stars/watches/notifications/invites)

crueber commented

2026-09-04 22:39:37 +00:00

Owner

Follow-up from the backend audit on issue #59 (origin/main @940ca8c).

Evidence: Registry.Delete (internal/wal/registry.go:271-318) sweeps only the repos/// prefix. These families live outside it and are never cleaned: users/

/starred|watching//.json (internal/social/social.go:148-161; internal/notify/notify.go:296-298), users/

/notifications/.json + index.json (internal/notify/notify.go:283-296), users/

/invitations/index.json (internal/identity/identity.go:153-155).

Impact: (1) Starred() lists with no repo-existence check (internal/social/service.go:187-217), so deleted repos linger in starred lists and dead notifications/invites accumulate. (2) On delete+recreate, the stale star record makes Star() take the early-return path (internal/social/service.go:32-36, no counter bump) while fresh meta/social.json starts at 0: user shows starred with desynced counters until an unstar/restar reconverges.

Suggested fix: lazy-existence filtering on the read paths (Starred/tray/inbox skip repos whose manifest is absent), or a delete-time tombstone the readers honor. orgs/* must keep surviving (shared by design).

Follow-up from the backend audit on issue #59 (origin/main @940ca8c). Evidence: Registry.Delete (internal/wal/registry.go:271-318) sweeps only the repos/<o>/<r>/ prefix. These families live outside it and are never cleaned: users/<p>/starred|watching/<o>/<r>.json (internal/social/social.go:148-161; internal/notify/notify.go:296-298), users/<p>/notifications/<id>.json + index.json (internal/notify/notify.go:283-296), users/<p>/invitations/index.json (internal/identity/identity.go:153-155). Impact: (1) Starred() lists with no repo-existence check (internal/social/service.go:187-217), so deleted repos linger in starred lists and dead notifications/invites accumulate. (2) On delete+recreate, the stale star record makes Star() take the early-return path (internal/social/service.go:32-36, no counter bump) while fresh meta/social.json starts at 0: user shows starred with desynced counters until an unstar/restar reconverges. Suggested fix: lazy-existence filtering on the read paths (Starred/tray/inbox skip repos whose manifest is absent), or a delete-time tombstone the readers honor. orgs/* must keep surviving (shared by design).

crueber referenced this issue from a commit

2026-09-04 22:56:08 +00:00

Fix #63: repo delete cleans userspace references via lazy tolerance

crueber referenced this issue from a pull request that will close it,

2026-09-04 22:56:11 +00:00

Fix #63: repo delete cleans userspace references #66

crueber commented

2026-09-04 22:56:15 +00:00

Author

Owner

Fix ready for review: PR #66 (branch fix/issue-63) — #66

Design (a)+(c): miss-tolerant reads everywhere (Starred/tray/inbox skip dead repos; Counts 404s; viewer/watch flags hide), counter resync on recreate (already-starred path repairs absent/zeroed counters with one +1), notifications for dead repos dropped by retention (window + bounded overflow sweep, unread_count reconciled). Writes fail closed (star/watch/invite-create 404, accept 409, unstar/unwatch clean without resurrecting social.json). Verified: gofmt/vet clean, -race green, cover 99.3/97.1/97.4%, Decisions entries in 07/06/01, E8 budgets updated. P6/auth untouched; deletion stays admin-only.

Fix ready for review: PR #66 (branch fix/issue-63) — https://git.packden.us/crueber/walhub/pulls/66 Design (a)+(c): miss-tolerant reads everywhere (Starred/tray/inbox skip dead repos; Counts 404s; viewer/watch flags hide), counter resync on recreate (already-starred path repairs absent/zeroed counters with one +1), notifications for dead repos dropped by retention (window + bounded overflow sweep, unread_count reconciled). Writes fail closed (star/watch/invite-create 404, accept 409, unstar/unwatch clean without resurrecting social.json). Verified: gofmt/vet clean, -race green, cover 99.3/97.1/97.4%, Decisions entries in 07/06/01, E8 budgets updated. P6/auth untouched; deletion stays admin-only.

crueber commented

2026-09-04 22:59:37 +00:00

Author

Owner

Review of PR #66 (79db5c4, fix/issue-63) — verified in scratch worktree, main worktree untouched. No changes pushed (no blocking problems found).

Checks (all PASS):

No reverse index: only new LIST is retainOverflow (internal/notify/tasks.go:465) on the per-user NotifPrefix, capped at 1000 scanned with the shared 200-delete budget. No users LIST anywhere; Starred LIST (internal/social/service.go:240) is the pre-existing per-user prefix; identity LISTs are pre-existing per-org/per-repo.
No oracle: deleted == never-existed on every path. Star ghost 404 (service.go:33), Counts ghost 404 (:176), CreateRepoInvite ghost 404 (invites.go), SetWatch-on ghost 404 (watch.go), AcceptInvite ghost 409 (invites.go:418) matching findInvite's existing swept-object 409. requireRead/P6 untouched so private-repo 403 behavior is unchanged. All repoAlive probes fail OPEN (true on store error) in social.go/notify.go/invites.go; malformed repos keep the entry. Starred/tray/inbox skip dead but still render private (no per-entry visibility check, pre-existing) — no new distinguisher.
Write codes via generic errors.Is statusFor in all three packages (ErrNotFound->404, ErrConflict->409); no handler changes needed. Unstar ghost deletes the record, returns (0,nil), skips the CAS so no social.json resurrection; bumpStars floors at 0 (service.go:131-134).
Resync: 412 race recounts without repairing (concurrent Star owns the bump); cross-user stale +1s converge (N records -> +N). Residual note (non-blocking): same-principal concurrent Star in the post-recreate zero window can double-bump (check-then-act in reconcileStar); millisecond window, heals via unstar/restar, same class as the documented known limit — no reverse index can fix it.
Retention: dead-repo rows dropped any-state + objects Deleted under the 200 cap; overflow sweep skips index.json, skips already-handled ids, never touches live-repo overflow; unread_count reconciled with dead ids in gone set. Tray filter runs pre-sort on the merged set (cursor pointing at a dropped dead entry resets to first page — harmless).
issuerAlive correct: CreateRepoInvite writes issuer object (PutCreate) BEFORE inboxAdd (CAS), so row-without-object means not-pending (cancel/sweep), never a live pending invite. Org-only rows unaffected (orgs/* survive by design). Expiry still lists + fails closed on accept, unchanged.
P6/auth untouched; no new imports (diff adds none); CAS discipline kept (Create-only records, version-conditional unstar delete, field-scoped CAS loops; repoAlive is a read-only HEAD probe, no blind PUTs).
Docs accurate: 07 4.1, 06 retention+Decisions, 01 Decisions, E8 +1 HEAD lines all match the code. Starred-list O(n) HEADs honestly declared as the #65 matter; this PR adds at most +1 HEAD/record and says so.

Tests (scratch worktree /tmp/pr66 @79db5c4): go test -race ./internal/social/... ./internal/notify/... ./internal/identity/... — all ok. Coverage: social 99.3%, notify 97.1%, identity 97.4% (gate >=95%). gofmt -l clean, go vet clean. Scratch worktree removed.

MERGE RECOMMENDATION: ready to merge.

Review of PR #66 (79db5c4, fix/issue-63) — verified in scratch worktree, main worktree untouched. No changes pushed (no blocking problems found). Checks (all PASS): - No reverse index: only new LIST is retainOverflow (internal/notify/tasks.go:465) on the per-user NotifPrefix, capped at 1000 scanned with the shared 200-delete budget. No users LIST anywhere; Starred LIST (internal/social/service.go:240) is the pre-existing per-user prefix; identity LISTs are pre-existing per-org/per-repo. - No oracle: deleted == never-existed on every path. Star ghost 404 (service.go:33), Counts ghost 404 (:176), CreateRepoInvite ghost 404 (invites.go), SetWatch-on ghost 404 (watch.go), AcceptInvite ghost 409 (invites.go:418) matching findInvite's existing swept-object 409. requireRead/P6 untouched so private-repo 403 behavior is unchanged. All repoAlive probes fail OPEN (true on store error) in social.go/notify.go/invites.go; malformed repos keep the entry. Starred/tray/inbox skip dead but still render private (no per-entry visibility check, pre-existing) — no new distinguisher. - Write codes via generic errors.Is statusFor in all three packages (ErrNotFound->404, ErrConflict->409); no handler changes needed. Unstar ghost deletes the record, returns (0,nil), skips the CAS so no social.json resurrection; bumpStars floors at 0 (service.go:131-134). - Resync: 412 race recounts without repairing (concurrent Star owns the bump); cross-user stale +1s converge (N records -> +N). Residual note (non-blocking): same-principal concurrent Star in the post-recreate zero window can double-bump (check-then-act in reconcileStar); millisecond window, heals via unstar/restar, same class as the documented known limit — no reverse index can fix it. - Retention: dead-repo rows dropped any-state + objects Deleted under the 200 cap; overflow sweep skips index.json, skips already-handled ids, never touches live-repo overflow; unread_count reconciled with dead ids in gone set. Tray filter runs pre-sort on the merged set (cursor pointing at a dropped dead entry resets to first page — harmless). - issuerAlive correct: CreateRepoInvite writes issuer object (PutCreate) BEFORE inboxAdd (CAS), so row-without-object means not-pending (cancel/sweep), never a live pending invite. Org-only rows unaffected (orgs/* survive by design). Expiry still lists + fails closed on accept, unchanged. - P6/auth untouched; no new imports (diff adds none); CAS discipline kept (Create-only records, version-conditional unstar delete, field-scoped CAS loops; repoAlive is a read-only HEAD probe, no blind PUTs). - Docs accurate: 07 4.1, 06 retention+Decisions, 01 Decisions, E8 +1 HEAD lines all match the code. Starred-list O(n) HEADs honestly declared as the #65 matter; this PR adds at most +1 HEAD/record and says so. Tests (scratch worktree /tmp/pr66 @79db5c4): go test -race ./internal/social/... ./internal/notify/... ./internal/identity/... — all ok. Coverage: social 99.3%, notify 97.1%, identity 97.4% (gate >=95%). gofmt -l clean, go vet clean. Scratch worktree removed. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-04 22:59:57 +00:00

crueber referenced this issue from a commit

2026-09-04 22:59:57 +00:00

Merge pull request 'Fix #63: repo delete cleans userspace references' (#66) from fix/issue-63 into main

crueber commented

2026-09-04 22:59:57 +00:00

Author

Owner

Fixed by PR #66 (review clean; coverage 99.3/97.1/97.4), merged. Closing.

crueber referenced this issue from a commit

2026-09-04 23:15:29 +00:00