code
Clone this repository
https://tangled.org/bretton.dev/coves
git@knot.bretton.dev:bretton.dev/coves
For self-hosted knots, clone URLs may differ based on your setup.
Fixes four issues identified in PR review:
**BUG 1 - Performance: Remove redundant database query**
- Removed duplicate GetByDID call in BlockCommunity service method
- ResolveCommunityIdentifier already verifies community exists
- Reduces block operations from 2 DB queries to 1
**BUG 2 - Performance: Move regex compilation to package level**
- Moved DID validation regex to package-level variable in block.go
- Prevents recompiling regex on every block/unblock request
- Eliminates unnecessary CPU overhead on hot path
**BUG 3 - DRY: Remove duplicated extractRKeyFromURI**
- Removed duplicate implementations in service.go and tests
- Now uses shared utils.ExtractRKeyFromURI function
- Single source of truth for AT-URI parsing logic
**P1 - Critical: Fix duplicate block race condition**
- Added ErrBlockAlreadyExists error type
- Returns 409 Conflict instead of 500 when PDS has block but AppView hasn't indexed yet
- Handles normal race in eventually-consistent flow gracefully
- Prevents double-click scenarios from appearing as server failures
All tests passing (33.2s runtime, 100% pass rate).
馃 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
**Breaking Change**: XRPC endpoints now strictly enforce lexicon spec.
Changed endpoints to reject handles and accept ONLY DIDs:
- social.coves.community.blockCommunity
- social.coves.community.unblockCommunity
- social.coves.community.subscribe
- social.coves.community.unsubscribe
Rationale:
1. Lexicon defines "subject" field with format: "did" (not "at-identifier")
2. Records are immutable and content-addressed - must use permanent DIDs
3. Handles can change (they're DNS pointers), DIDs cannot
4. Bluesky's app.bsky.graph.block uses same pattern (DID-only)
Previous behavior accepted both DIDs and handles, resolving handles to
DIDs internally. This was convenient but violated the lexicon contract.
Impact:
- Clients must resolve handles to DIDs before calling these endpoints
- Matches standard atProto patterns for block/subscription records
- Ensures federation compatibility
This aligns our implementation with the lexicon specification and
atProto best practices.
Improve validation robustness in block/unblock handlers:
1. DID validation with regex:
- Pattern: ^did:(plc|web):[a-zA-Z0-9._:%-]+$
- Rejects invalid formats like "did:x" or "did:"
- Ensures only supported DID methods (plc, web)
2. Handle validation:
- Verify handle contains @ symbol for domain
- Rejects incomplete handles like "!" or "!name"
- Ensures proper format: !name@domain.tld
Previous validation only checked prefix, allowing invalid values
to pass through to service layer. New validation catches format
errors early with clear error messages.
Addresses: Important review comment #4
Service layer improvements:
1. Add DID verification in ResolveCommunityIdentifier:
- When a DID is provided, verify the community actually exists
in the AppView database
- Prevents accepting non-existent DIDs (e.g., did:plc:fakefake)
- Provides clearer error messages when community doesn't exist
2. Improve duplicate error detection in BlockCommunity:
- Check for HTTP 409 Conflict status code explicitly
- Added "status 409" check in addition to text-based detection
- More robust across different PDS implementations
- Still maintains fallback checks for compatibility
Both changes improve error handling and user experience while
maintaining backward compatibility.
Addresses: Critical review comment #2, Important review comment #3
Database optimization changes:
1. Removed redundant idx_blocks_user_community index:
- UNIQUE constraint on (user_did, community_did) already creates
an index automatically
- Maintaining duplicate index wastes storage and degrades write
performance (every insert updates two identical indexes)
2. Added missing idx_blocks_record_uri index:
- Required for GetBlockByURI() queries used in Jetstream DELETE
operations
- Without this index, DELETE event processing does full table scan
Migration now has optimal indexes without redundancy.
Addresses: Critical review comments #1 and #7
Critical bug fix: The loop variable 'block' was being reused for each
iteration, causing all elements in the returned slice to point to the
same memory location. This resulted in the last row being repeated for
every element when callers read the list.
Fixed by allocating a new block pointer for each iteration:
- Before: var block communities.CommunityBlock (reused)
- After: block := &communities.CommunityBlock{} (new allocation)
Also replaced fmt.Printf with log.Printf for consistency with project
logging standards.
Addresses: P1 review comment - pointer reuse in list operation