big scary refactor. sync enable_bluesky_posts with captain record. implement oauth logout handler. implement crew assignment to hold. this caused a lot of circular dependencies and needed to move functions around in order to fix

pkg/hold/config.go

@@ -40,7 +40,7 @@ type RegistrationConfig struct {
 
 	// EnableBlueskyPosts controls whether to create Bluesky posts for manifest uploads (from env: HOLD_BLUESKY_POSTS_ENABLED)
 	// If true, creates posts when users push images
-	// Can be overridden per-hold via captain record's enableManifestPosts field
+	// Synced to captain record's enableBlueskyPosts field on startup
 	EnableBlueskyPosts bool `yaml:"enable_bluesky_posts"`
 }
 

pkg/hold/oci/xrpc.go

@@ -244,9 +244,15 @@ func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Reques
 	}
 
 	// Check if manifest posts are enabled
-	// Controlled by HOLD_BLUESKY_POSTS_ENABLED environment variable
-	// TODO: Override with captain record enableManifestPosts field if set
-	postsEnabled := h.enableBlueskyPosts
+	// Read from captain record (which is synced with HOLD_BLUESKY_POSTS_ENABLED env var)
+	postsEnabled := false
+	_, captain, err := h.pds.GetCaptainRecord(ctx)
+	if err == nil {
+		postsEnabled = captain.EnableBlueskyPosts
+	} else {
+		// Fallback to env var if captain record doesn't exist (shouldn't happen in normal operation)
+		postsEnabled = h.enableBlueskyPosts
+	}
 
 	// Create layer records for each blob
 	layersCreated := 0

pkg/hold/pds/auth_test.go

@@ -742,7 +742,7 @@ func TestValidateBlobReadAccess_PrivateHold(t *testing.T) {
 	pds, ctx := setupTestPDSWithBootstrap(t, ownerDID, false, false)
 
 	// Update captain to be private
-	_, err := pds.UpdateCaptainRecord(ctx, false, false)
+	_, err := pds.UpdateCaptainRecord(ctx, false, false, false)
 	if err != nil {
 		t.Fatalf("Failed to update captain record: %v", err)
 	}

pkg/hold/pds/captain.go

@@ -16,13 +16,14 @@ const (
 
 // CreateCaptainRecord creates the captain record for the hold (first-time only).
 // This will FAIL if the captain record already exists. Use UpdateCaptainRecord to modify.
-func (p *HoldPDS) CreateCaptainRecord(ctx context.Context, ownerDID string, public bool, allowAllCrew bool) (cid.Cid, error) {
+func (p *HoldPDS) CreateCaptainRecord(ctx context.Context, ownerDID string, public bool, allowAllCrew bool, enableBlueskyPosts bool) (cid.Cid, error) {
 	captainRecord := &atproto.CaptainRecord{
-		Type:         atproto.CaptainCollection,
-		Owner:        ownerDID,
-		Public:       public,
-		AllowAllCrew: allowAllCrew,
-		DeployedAt:   time.Now().Format(time.RFC3339),
+		Type:               atproto.CaptainCollection,
+		Owner:              ownerDID,
+		Public:             public,
+		AllowAllCrew:       allowAllCrew,
+		EnableBlueskyPosts: enableBlueskyPosts,
+		DeployedAt:         time.Now().Format(time.RFC3339),
 	}
 
 	// Use repomgr.PutRecord - creates with explicit rkey, fails if already exists
@@ -53,8 +54,8 @@ func (p *HoldPDS) GetCaptainRecord(ctx context.Context) (cid.Cid, *atproto.Capta
 	return recordCID, captainRecord, nil
 }
 
-// UpdateCaptainRecord updates the captain record (e.g., to change public/allowAllCrew settings)
-func (p *HoldPDS) UpdateCaptainRecord(ctx context.Context, public bool, allowAllCrew bool) (cid.Cid, error) {
+// UpdateCaptainRecord updates the captain record (e.g., to change public/allowAllCrew/enableBlueskyPosts settings)
+func (p *HoldPDS) UpdateCaptainRecord(ctx context.Context, public bool, allowAllCrew bool, enableBlueskyPosts bool) (cid.Cid, error) {
 	// Get existing record to preserve other fields
 	_, existing, err := p.GetCaptainRecord(ctx)
 	if err != nil {
@@ -64,6 +65,7 @@ func (p *HoldPDS) UpdateCaptainRecord(ctx context.Context, public bool, allowAll
 	// Update the fields
 	existing.Public = public
 	existing.AllowAllCrew = allowAllCrew
+	existing.EnableBlueskyPosts = enableBlueskyPosts
 
 	recordCID, err := p.repomgr.UpdateRecord(ctx, p.uid, atproto.CaptainCollection, CaptainRkey, existing)
 	if err != nil {

pkg/hold/pds/captain_test.go

@@ -71,34 +71,39 @@ func setupTestPDSWithBootstrap(t *testing.T, ownerDID string, public, allowAllCr
 // TestCreateCaptainRecord tests creating a captain record with various settings
 func TestCreateCaptainRecord(t *testing.T) {
 	tests := []struct {
-		name         string
-		ownerDID     string
-		public       bool
-		allowAllCrew bool
+		name               string
+		ownerDID           string
+		public             bool
+		allowAllCrew       bool
+		enableBlueskyPosts bool
 	}{
 		{
-			name:         "Private hold, no all-crew",
-			ownerDID:     "did:plc:alice123",
-			public:       false,
-			allowAllCrew: false,
+			name:               "Private hold, no all-crew",
+			ownerDID:           "did:plc:alice123",
+			public:             false,
+			allowAllCrew:       false,
+			enableBlueskyPosts: false,
 		},
 		{
-			name:         "Public hold, no all-crew",
-			ownerDID:     "did:plc:bob456",
-			public:       true,
-			allowAllCrew: false,
+			name:               "Public hold, no all-crew",
+			ownerDID:           "did:plc:bob456",
+			public:             true,
+			allowAllCrew:       false,
+			enableBlueskyPosts: true,
 		},
 		{
-			name:         "Public hold, allow all crew",
-			ownerDID:     "did:plc:charlie789",
-			public:       true,
-			allowAllCrew: true,
+			name:               "Public hold, allow all crew",
+			ownerDID:           "did:plc:charlie789",
+			public:             true,
+			allowAllCrew:       true,
+			enableBlueskyPosts: false,
 		},
 		{
-			name:         "Private hold, allow all crew",
-			ownerDID:     "did:plc:dave012",
-			public:       false,
-			allowAllCrew: true,
+			name:               "Private hold, allow all crew",
+			ownerDID:           "did:plc:dave012",
+			public:             false,
+			allowAllCrew:       true,
+			enableBlueskyPosts: true,
 		},
 	}
 
@@ -109,7 +114,7 @@ func TestCreateCaptainRecord(t *testing.T) {
 			defer pds.Close()
 
 			// Create captain record
-			recordCID, err := pds.CreateCaptainRecord(ctx, tt.ownerDID, tt.public, tt.allowAllCrew)
+			recordCID, err := pds.CreateCaptainRecord(ctx, tt.ownerDID, tt.public, tt.allowAllCrew, tt.enableBlueskyPosts)
 			if err != nil {
 				t.Fatalf("CreateCaptainRecord failed: %v", err)
 			}
@@ -138,6 +143,9 @@ func TestCreateCaptainRecord(t *testing.T) {
 			if captain.AllowAllCrew != tt.allowAllCrew {
 				t.Errorf("Expected allowAllCrew=%v, got %v", tt.allowAllCrew, captain.AllowAllCrew)
 			}
+			if captain.EnableBlueskyPosts != tt.enableBlueskyPosts {
+				t.Errorf("Expected enableBlueskyPosts=%v, got %v", tt.enableBlueskyPosts, captain.EnableBlueskyPosts)
+			}
 			if captain.Type != atproto.CaptainCollection {
 				t.Errorf("Expected type %s, got %s", atproto.CaptainCollection, captain.Type)
 			}
@@ -156,7 +164,7 @@ func TestGetCaptainRecord(t *testing.T) {
 	ownerDID := "did:plc:alice123"
 
 	// Create captain record
-	createdCID, err := pds.CreateCaptainRecord(ctx, ownerDID, true, false)
+	createdCID, err := pds.CreateCaptainRecord(ctx, ownerDID, true, false, false)
 	if err != nil {
 		t.Fatalf("CreateCaptainRecord failed: %v", err)
 	}
@@ -212,8 +220,8 @@ func TestUpdateCaptainRecord(t *testing.T) {
 
 	ownerDID := "did:plc:alice123"
 
-	// Create initial captain record (public=false, allowAllCrew=false)
-	_, err := pds.CreateCaptainRecord(ctx, ownerDID, false, false)
+	// Create initial captain record (public=false, allowAllCrew=false, enableBlueskyPosts=false)
+	_, err := pds.CreateCaptainRecord(ctx, ownerDID, false, false, false)
 	if err != nil {
 		t.Fatalf("CreateCaptainRecord failed: %v", err)
 	}
@@ -231,9 +239,12 @@ func TestUpdateCaptainRecord(t *testing.T) {
 	if captain1.AllowAllCrew {
 		t.Error("Expected initial allowAllCrew=false")
 	}
+	if captain1.EnableBlueskyPosts {
+		t.Error("Expected initial enableBlueskyPosts=false")
+	}
 
-	// Update to public=true, allowAllCrew=true
-	updatedCID, err := pds.UpdateCaptainRecord(ctx, true, true)
+	// Update to public=true, allowAllCrew=true, enableBlueskyPosts=true
+	updatedCID, err := pds.UpdateCaptainRecord(ctx, true, true, true)
 	if err != nil {
 		t.Fatalf("UpdateCaptainRecord failed: %v", err)
 	}
@@ -260,14 +271,17 @@ func TestUpdateCaptainRecord(t *testing.T) {
 	if !captain2.AllowAllCrew {
 		t.Error("Expected allowAllCrew=true after update")
 	}
+	if !captain2.EnableBlueskyPosts {
+		t.Error("Expected enableBlueskyPosts=true after update")
+	}
 
 	// Verify owner didn't change
 	if captain2.Owner != ownerDID {
 		t.Errorf("Expected owner to remain %s, got %s", ownerDID, captain2.Owner)
 	}
 
-	// Update again to different values (public=true, allowAllCrew=false)
-	_, err = pds.UpdateCaptainRecord(ctx, true, false)
+	// Update again to different values (public=true, allowAllCrew=false, enableBlueskyPosts=false)
+	_, err = pds.UpdateCaptainRecord(ctx, true, false, false)
 	if err != nil {
 		t.Fatalf("Second UpdateCaptainRecord failed: %v", err)
 	}
@@ -292,7 +306,7 @@ func TestUpdateCaptainRecord_NotFound(t *testing.T) {
 	defer pds.Close()
 
 	// Try to update captain record before creating one
-	_, err := pds.UpdateCaptainRecord(ctx, true, true)
+	_, err := pds.UpdateCaptainRecord(ctx, true, true, true)
 	if err == nil {
 		t.Fatal("Expected error when updating non-existent captain record")
 	}

pkg/hold/pds/server.go

@@ -155,12 +155,12 @@ func (p *HoldPDS) Bootstrap(ctx context.Context, storageDriver driver.StorageDri
 		}
 
 		// Create captain record (hold ownership and settings)
-		_, err = p.CreateCaptainRecord(ctx, ownerDID, public, allowAllCrew)
+		_, err = p.CreateCaptainRecord(ctx, ownerDID, public, allowAllCrew, p.enableBlueskyPosts)
 		if err != nil {
 			return fmt.Errorf("failed to create captain record: %w", err)
 		}
 
-		fmt.Printf("✅ Created captain record (public=%v, allowAllCrew=%v)\n", public, allowAllCrew)
+		fmt.Printf("✅ Created captain record (public=%v, allowAllCrew=%v, enableBlueskyPosts=%v)\n", public, allowAllCrew, p.enableBlueskyPosts)
 
 		// Add hold owner as first crew member with admin role
 		_, err = p.AddCrewMember(ctx, ownerDID, "admin", []string{"blob:read", "blob:write", "crew:admin"})
@@ -169,6 +169,24 @@ func (p *HoldPDS) Bootstrap(ctx context.Context, storageDriver driver.StorageDri
 		}
 
 		fmt.Printf("✅ Added %s as hold admin\n", ownerDID)
+	} else {
+		// Captain record exists, check if we need to sync settings from env vars
+		_, existingCaptain, err := p.GetCaptainRecord(ctx)
+		if err == nil {
+			// Check if any settings need updating
+			needsUpdate := existingCaptain.Public != public ||
+				existingCaptain.AllowAllCrew != allowAllCrew ||
+				existingCaptain.EnableBlueskyPosts != p.enableBlueskyPosts
+
+			if needsUpdate {
+				// Update captain record to match env vars
+				_, err = p.UpdateCaptainRecord(ctx, public, allowAllCrew, p.enableBlueskyPosts)
+				if err != nil {
+					return fmt.Errorf("failed to update captain record: %w", err)
+				}
+				fmt.Printf("✅ Synced captain record with env vars (public=%v, allowAllCrew=%v, enableBlueskyPosts=%v)\n", public, allowAllCrew, p.enableBlueskyPosts)
+			}
+		}
 	}
 
 	// Create Bluesky profile record (idempotent - check if exists first)

pkg/hold/pds/server_test.go

@@ -560,7 +560,7 @@ func TestBootstrap_CaptainWithoutCrew(t *testing.T) {
 
 	// Create captain record WITHOUT crew (unusual state)
 	ownerDID := "did:plc:alice123"
-	_, err = pds.CreateCaptainRecord(ctx, ownerDID, true, false)
+	_, err = pds.CreateCaptainRecord(ctx, ownerDID, true, false, false)
 	if err != nil {
 		t.Fatalf("CreateCaptainRecord failed: %v", err)
 	}

pkg/hold/pds/xrpc_test.go

@@ -1199,7 +1199,7 @@ func TestHandleRequestCrew(t *testing.T) {
 	handler, ctx := setupTestXRPCHandler(t)
 
 	// Update captain record to allow all crew
-	_, err := handler.pds.UpdateCaptainRecord(ctx, true, true) // public=true, allowAllCrew=true
+	_, err := handler.pds.UpdateCaptainRecord(ctx, true, true, false) // public=true, allowAllCrew=true, enableBlueskyPosts=false
 	if err != nil {
 		t.Fatalf("Failed to update captain record: %v", err)
 	}
@@ -1243,7 +1243,7 @@ func TestHandleRequestCrew_AllowAllCrewDisabled(t *testing.T) {
 
 	// Captain record was created with allowAllCrew=false in setupTestXRPCHandler
 	// Update to make sure it's false
-	_, err := handler.pds.UpdateCaptainRecord(ctx, true, false) // public=true, allowAllCrew=false
+	_, err := handler.pds.UpdateCaptainRecord(ctx, true, false, false) // public=true, allowAllCrew=false, enableBlueskyPosts=false
 	if err != nil {
 		t.Fatalf("Failed to update captain record: %v", err)
 	}
@@ -1715,7 +1715,7 @@ func TestHandleGetBlob_CORSHeaders(t *testing.T) {
 	handler, _, ctx := setupTestXRPCHandlerWithBlobs(t)
 
 	// Make hold public
-	_, err := handler.pds.UpdateCaptainRecord(ctx, true, false)
+	_, err := handler.pds.UpdateCaptainRecord(ctx, true, false, false)
 	if err != nil {
 		t.Fatalf("Failed to update captain: %v", err)
 	}