From 606c8a842a1dd25314a8f3b667611f5ce87c3e99 Mon Sep 17 00:00:00 2001
From: Evan Jarrett <evan@evanjarrett.com>
Date: Fri, 17 Oct 2025 17:42:23 -0500
Subject: [PATCH] minor bug fixes around hold did:web instead of url endpoint

---
 pkg/appview/templates/pages/settings.html | 22 ++++++++++--
 pkg/atproto/lexicon.go                    |  6 ++++
 pkg/atproto/profile.go                    | 36 +++++++++++++++++--
 pkg/hold/service.go                       | 44 -----------------------
 pkg/hold/storage.go                       | 20 +++--------
 5 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/pkg/appview/templates/pages/settings.html b/pkg/appview/templates/pages/settings.html
index 8e79a8d..4fd12d3 100644
--- a/pkg/appview/templates/pages/settings.html
+++ b/pkg/appview/templates/pages/settings.html
@@ -35,11 +35,12 @@
     <!-- Default Hold Section -->
     <section class="settings-section">
         <h2>Default Hold</h2>
-        <p>Current: <strong>{{ if .Profile.DefaultHold }}{{ .Profile.DefaultHold }}{{ else }}Not set{{ end }}</strong></p>
+        <p>Current: <strong id="current-hold">{{ if .Profile.DefaultHold }}{{ .Profile.DefaultHold }}{{ else }}Not set{{ end }}</strong></p>
 
         <form hx-post="/api/profile/default-hold"
               hx-target="#hold-status"
-              hx-swap="innerHTML">
+              hx-swap="innerHTML"
+              id="hold-form">
 
             <div class="form-group">
                 <label for="hold-endpoint">Hold Endpoint:</label>
@@ -115,6 +116,23 @@
     <script src="/static/js/app.js"></script>
 
     <script>
+    // Default Hold Update - Dynamic display update
+    document.addEventListener('DOMContentLoaded', function() {
+        const holdForm = document.getElementById('hold-form');
+
+        holdForm.addEventListener('htmx:afterSwap', function(event) {
+            // Check if the response contains success indicator
+            if (event.detail.xhr.status === 200) {
+                const holdInput = document.getElementById('hold-endpoint');
+                const currentHoldDisplay = document.getElementById('current-hold');
+                const newValue = holdInput.value.trim();
+
+                // Update the current hold display
+                currentHoldDisplay.textContent = newValue || 'Not set';
+            }
+        });
+    });
+
     // Device Management JavaScript
     (function() {
         // Load devices
diff --git a/pkg/atproto/lexicon.go b/pkg/atproto/lexicon.go
index d2510ec..4d1becf 100644
--- a/pkg/atproto/lexicon.go
+++ b/pkg/atproto/lexicon.go
@@ -364,12 +364,18 @@ func ParseStarRecordKey(rkey string) (ownerDID, repository string, err error) {
 
 // ResolveHoldDIDFromURL converts a hold endpoint URL to a did:web DID
 // For did:web holds: https://hold01.atcr.io → did:web:hold01.atcr.io
+// If input is already a DID, returns it as-is
 func ResolveHoldDIDFromURL(holdURL string) string {
 	// Handle empty URLs
 	if holdURL == "" {
 		return ""
 	}
 
+	// If already a DID, return as-is
+	if strings.HasPrefix(holdURL, "did:") {
+		return holdURL
+	}
+
 	// Parse URL to get hostname
 	holdURL = strings.TrimPrefix(holdURL, "http://")
 	holdURL = strings.TrimPrefix(holdURL, "https://")
diff --git a/pkg/atproto/profile.go b/pkg/atproto/profile.go
index 4fdb078..dcf5db2 100644
--- a/pkg/atproto/profile.go
+++ b/pkg/atproto/profile.go
@@ -5,11 +5,17 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sync"
+	"time"
 )
 
 // Profile record key is always "self" per lexicon
 const ProfileRKey = "self"
 
+// Global map to track in-flight profile migrations (DID -> true)
+// Used to prevent duplicate migration goroutines
+var migrationLocks sync.Map
+
 // EnsureProfile checks if a user's profile exists and creates it if needed
 // This should be called during authentication (OAuth exchange or token service)
 // If defaultHoldDID is provided, creates profile with that default (or empty if not provided)
@@ -65,8 +71,34 @@ func GetProfile(ctx context.Context, client *Client) (*SailorProfileRecord, erro
 	// This ensures backward compatibility with profiles created before DID migration
 	if profile.DefaultHold != "" && !isDID(profile.DefaultHold) {
 		// Convert URL to DID transparently
-		profile.DefaultHold = ResolveHoldDIDFromURL(profile.DefaultHold)
-		fmt.Printf("DEBUG [profile]: Migrated defaultHold URL to DID: %s\n", profile.DefaultHold)
+		migratedDID := ResolveHoldDIDFromURL(profile.DefaultHold)
+		profile.DefaultHold = migratedDID
+
+		// Persist the migration to PDS in a background goroutine
+		// Use a lock to ensure only one goroutine migrates this DID
+		did := client.did
+		if _, loaded := migrationLocks.LoadOrStore(did, true); !loaded {
+			// We got the lock - launch goroutine to persist the migration
+			go func() {
+				// Clean up lock when done (after a short delay to batch requests)
+				defer func() {
+					time.Sleep(1 * time.Second)
+					migrationLocks.Delete(did)
+				}()
+
+				// Create a new context with timeout for the background operation
+				ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+				defer cancel()
+
+				// Update the profile on the PDS
+				profile.UpdatedAt = time.Now()
+				if err := UpdateProfile(ctx, client, &profile); err != nil {
+					fmt.Printf("WARNING [profile]: Failed to persist URL-to-DID migration for %s: %v\n", did, err)
+				} else {
+					fmt.Printf("DEBUG [profile]: Persisted defaultHold migration to DID: %s (for DID: %s)\n", migratedDID, did)
+				}
+			}()
+		}
 	}
 
 	return &profile, nil
diff --git a/pkg/hold/service.go b/pkg/hold/service.go
index eff7413..84840bd 100644
--- a/pkg/hold/service.go
+++ b/pkg/hold/service.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"net/url"
 
 	"atcr.io/pkg/auth"
 	"github.com/aws/aws-sdk-go/service/s3"
@@ -74,46 +73,3 @@ func NewHoldService(cfg *Config, holdPDS any) (*HoldService, error) {
 
 	return service, nil
 }
-
-// GetPresignedURL is a public wrapper around getPresignedURL for use by PDS blob store
-func (s *HoldService) GetPresignedURL(ctx context.Context, operation PresignedURLOperation, digest string, did string) (string, error) {
-	return s.getPresignedURL(ctx, operation, digest, did)
-}
-
-// isAuthorizedRead checks if the given DID has read access to this hold
-// This is a helper wrapper around the authorizer for internal use
-func (s *HoldService) isAuthorizedRead(did string) bool {
-	ctx := context.Background()
-	allowed, err := s.authorizer.CheckReadAccess(ctx, s.pds.DID(), did)
-	if err != nil {
-		log.Printf("Authorization check failed: %v", err)
-		return false
-	}
-	return allowed
-}
-
-// isAuthorizedWrite checks if the given DID has write access to this hold
-// This is a helper wrapper around the authorizer for internal use
-func (s *HoldService) isAuthorizedWrite(did string) bool {
-	ctx := context.Background()
-	allowed, err := s.authorizer.CheckWriteAccess(ctx, s.pds.DID(), did)
-	if err != nil {
-		log.Printf("Authorization check failed: %v", err)
-		return false
-	}
-	return allowed
-}
-
-// extractHostname extracts the hostname from a URL
-func extractHostname(urlStr string) (string, error) {
-	u, err := url.Parse(urlStr)
-	if err != nil {
-		return "", err
-	}
-	// Remove port if present
-	hostname := u.Hostname()
-	if hostname == "" {
-		return "", fmt.Errorf("no hostname in URL")
-	}
-	return hostname, nil
-}
diff --git a/pkg/hold/storage.go b/pkg/hold/storage.go
index 5e7cdca..d3b8ad7 100644
--- a/pkg/hold/storage.go
+++ b/pkg/hold/storage.go
@@ -9,6 +9,8 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/s3"
+
+	"atcr.io/pkg/atproto"
 )
 
 // atprotoBlobPath creates a per-DID storage path for ATProto blobs
@@ -51,7 +53,7 @@ func blobPath(digest string) string {
 
 // getPresignedURL generates a presigned URL for GET, HEAD, or PUT operations
 // Distinguishes between ATProto blobs (per-DID) and OCI blobs (content-addressed)
-func (s *HoldService) getPresignedURL(ctx context.Context, operation PresignedURLOperation, digest string, did string) (string, error) {
+func (s *HoldService) GetPresignedURL(ctx context.Context, operation PresignedURLOperation, digest string, did string) (string, error) {
 	var path string
 
 	// Determine blob type and construct appropriate path
@@ -151,8 +153,8 @@ func (s *HoldService) getPresignedURL(ctx context.Context, operation PresignedUR
 func (s *HoldService) getProxyURL(digest, did string, operation PresignedURLOperation) string {
 	// For read operations, use XRPC getBlob endpoint
 	if operation == OperationGet || operation == OperationHead {
-		// Generate hold DID from public URL
-		holdDID := s.getHoldDID()
+		// Generate hold DID from public URL using shared function
+		holdDID := atproto.ResolveHoldDIDFromURL(s.config.Server.PublicURL)
 		return fmt.Sprintf("%s/xrpc/com.atproto.sync.getBlob?did=%s&cid=%s",
 			s.config.Server.PublicURL, holdDID, digest)
 	}
@@ -161,15 +163,3 @@ func (s *HoldService) getProxyURL(digest, did string, operation PresignedURLOper
 	// Clients should use multipart upload flow via com.atproto.repo.uploadBlob
 	return ""
 }
-
-// getHoldDID generates a did:web from the hold's public URL
-func (s *HoldService) getHoldDID() string {
-	// Convert URL to did:web format
-	// https://hold01.atcr.io → did:web:hold01.atcr.io
-	url := s.config.Server.PublicURL
-	url = strings.TrimPrefix(url, "https://")
-	url = strings.TrimPrefix(url, "http://")
-	url = strings.Split(url, "/")[0] // Remove path
-	url = strings.Split(url, ":")[0] // Remove port
-	return fmt.Sprintf("did:web:%s", url)
-}