bretton.dev
A community based topic aggregation platform built on atproto
docs: add PutRecord implementation backlog items
Add two P3 technical debt items for future optimistic locking:
1. Implement PutRecord in PDS Client
- Add swapRecord/swapCommit for optimistic locking
- Handle conflict responses gracefully
2. Migrate UpdateComment to Use PutRecord
- Blocked by PutRecord implementation
- Will prevent concurrent update races
These items are not urgent since concurrent comment updates are rare,
but will improve robustness when implemented.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
+124
docs/PRD_BACKLOG.md
+124
docs/PRD_BACKLOG.md
···
649
649
650
650
## 🔵 P3: Technical Debt
651
651
652
+
### Implement PutRecord in PDS Client
653
+
**Added:** 2025-12-04 | **Effort:** 2-3 hours | **Priority:** Technical Debt
654
+
**Status:** 📋 TODO
655
+
656
+
**Problem:**
657
+
The PDS client (`internal/atproto/pds/client.go`) only has `CreateRecord` but lacks `PutRecord`. This means updates use `CreateRecord` with an existing rkey, which:
658
+
1. Loses optimistic locking (no CID swap check)
659
+
2. Is semantically incorrect (creates vs updates)
660
+
3. Could cause race conditions on concurrent updates
661
+
662
+
**atProto Best Practice:**
663
+
- `com.atproto.repo.putRecord` should be used for updates
664
+
- Accepts `swapRecord` (expected CID) for optimistic locking
665
+
- Returns conflict error if CID doesn't match (concurrent modification detected)
666
+
667
+
**Solution:**
668
+
Add `PutRecord` method to the PDS client interface:
669
+
670
+
```go
671
+
// Client interface addition
672
+
type Client interface {
673
+
// ... existing methods ...
674
+
675
+
// PutRecord creates or updates a record with optional optimistic locking.
676
+
// If swapRecord is provided, the operation fails if the current CID doesn't match.
677
+
PutRecord(ctx context.Context, collection string, rkey string, record any, swapRecord string) (uri string, cid string, err error)
678
+
}
679
+
680
+
// Implementation
681
+
func (c *client) PutRecord(ctx context.Context, collection string, rkey string, record any, swapRecord string) (string, string, error) {
682
+
payload := map[string]any{
683
+
"repo": c.did,
684
+
"collection": collection,
685
+
"rkey": rkey,
686
+
"record": record,
687
+
}
688
+
689
+
// Optional: optimistic locking via CID swap check
690
+
if swapRecord != "" {
691
+
payload["swapRecord"] = swapRecord
692
+
}
693
+
694
+
var result struct {
695
+
URI string `json:"uri"`
696
+
CID string `json:"cid"`
697
+
}
698
+
699
+
err := c.apiClient.Post(ctx, syntax.NSID("com.atproto.repo.putRecord"), payload, &result)
700
+
if err != nil {
701
+
return "", "", wrapAPIError(err, "putRecord")
702
+
}
703
+
704
+
return result.URI, result.CID, nil
705
+
}
706
+
```
707
+
708
+
**Error Handling:**
709
+
Add new error type for conflict detection:
710
+
```go
711
+
var ErrConflict = errors.New("record was modified by another operation")
712
+
```
713
+
714
+
Map HTTP 409 in `wrapAPIError`:
715
+
```go
716
+
case 409:
717
+
return fmt.Errorf("%s: %w: %s", operation, ErrConflict, apiErr.Message)
718
+
```
719
+
720
+
**Files to Modify:**
721
+
- `internal/atproto/pds/client.go` - Add `PutRecord` method and interface
722
+
- `internal/atproto/pds/errors.go` - Add `ErrConflict` error type
723
+
724
+
**Testing:**
725
+
- Unit test: Verify payload includes `swapRecord` when provided
726
+
- Integration test: Concurrent updates detect conflict
727
+
- Integration test: Update without `swapRecord` still works (backwards compatible)
728
+
729
+
**Blocked By:** Nothing
730
+
**Blocks:** "Migrate UpdateComment to use PutRecord"
731
+
732
+
---
733
+
734
+
### Migrate UpdateComment to Use PutRecord
735
+
**Added:** 2025-12-04 | **Effort:** 1 hour | **Priority:** Technical Debt
736
+
**Status:** 📋 TODO (Blocked)
737
+
**Blocked By:** "Implement PutRecord in PDS Client"
738
+
739
+
**Problem:**
740
+
`UpdateComment` in `internal/core/comments/comment_service.go` uses `CreateRecord` for updates instead of `PutRecord`. This lacks optimistic locking and is semantically incorrect.
741
+
742
+
**Current Code (lines 687-690):**
743
+
```go
744
+
// TODO: Use PutRecord instead of CreateRecord for proper update semantics with optimistic locking.
745
+
// PutRecord should accept the existing CID (existingRecord.CID) to ensure concurrent updates are detected.
746
+
// However, PutRecord is not yet implemented in internal/atproto/pds/client.go.
747
+
uri, cid, err := pdsClient.CreateRecord(ctx, commentCollection, rkey, updatedRecord)
748
+
```
749
+
750
+
**Solution:**
751
+
Once `PutRecord` is implemented in the PDS client, update to:
752
+
```go
753
+
// Use PutRecord with optimistic locking via existing CID
754
+
uri, cid, err := pdsClient.PutRecord(ctx, commentCollection, rkey, updatedRecord, existingRecord.CID)
755
+
if err != nil {
756
+
if errors.Is(err, pds.ErrConflict) {
757
+
// Record was modified by another operation - return appropriate error
758
+
return nil, fmt.Errorf("comment was modified, please refresh and try again: %w", err)
759
+
}
760
+
// ... existing error handling
761
+
}
762
+
```
763
+
764
+
**Files to Modify:**
765
+
- `internal/core/comments/comment_service.go` - UpdateComment method
766
+
- `internal/core/comments/errors.go` - Add `ErrConcurrentModification` if needed
767
+
768
+
**Testing:**
769
+
- Unit test: Verify `PutRecord` is called with correct CID
770
+
- Integration test: Simulate concurrent update, verify conflict handling
771
+
772
+
**Impact:** Proper optimistic locking prevents lost updates from race conditions
773
+
774
+
---
775
+
652
776
### Consolidate Environment Variable Validation
653
777
**Added:** 2025-10-11 | **Effort:** 2-3 hours
654
778