Fix domain-assignment dead-ends and manual-provider honesty

internal/domain.go

@@ -91,13 +91,44 @@ func RegisterDomainEndpoints(pb *pocketbase.PocketBase) error {
 			return e.JSON(200, domainResponse(site, result))
 		})
 
+		// Manually mark a site's attached custom domain as live. This is the
+		// confirmation step for the manual (self-hosted) provider: Primo can't
+		// verify an external domain remotely, so once the operator has pointed
+		// DNS at the box and fronted it with TLS, they click "Mark as connected"
+		// and we flip the status to live. Guarded to the manual provider — a
+		// Railway domain's liveness is driven by the real cert status, so faking
+		// it there would lie about whether the cert actually issued.
+		serveEvent.Router.POST("/api/primo/sites/{siteId}/domain/mark-live", func(e *core.RequestEvent) error {
+			site, err := authorizeSiteDomain(pb, e)
+			if err != nil {
+				return err
+			}
+			if getDomainProvider().Name() != "manual" {
+				return e.BadRequestError("This domain's status is managed automatically.", nil)
+			}
+			host := site.GetString("host")
+			if host == "" || host == site.Id {
+				return e.BadRequestError("No custom domain is assigned to this site.", nil)
+			}
+			// Live + no records: a manual domain has no Primo-generated DNS
+			// records to track, and the operator has confirmed reachability.
+			if err := applyDomainResult(pb, site, host, DomainResult{Status: DomainStatusLive}); err != nil {
+				return e.InternalServerError("Failed to update domain status", err)
+			}
+			return e.JSON(200, domainResponse(site, DomainResult{Status: DomainStatusLive}))
+		})
+
 		// Re-check the status of a site's attached custom domain.
 		serveEvent.Router.GET("/api/primo/sites/{siteId}/domain/status", func(e *core.RequestEvent) error {
 			site, err := authorizeSiteDomain(pb, e)
 			if err != nil {
 				return err
 			}
 
+			if result, ok := domainStatusOverride(site); ok {
+				return e.JSON(200, domainResponse(site, result))
+			}
+
 			result, err := getDomainProvider().DomainStatus(site.GetString("domain_provider_id"), site.GetString("host"))
 			if err != nil {
 				return e.BadRequestError("Failed to check domain status: "+err.Error(), err)
@@ -142,6 +173,21 @@ func authorizeSiteDomain(pb *pocketbase.PocketBase, e *core.RequestEvent) (*core
 	return site, nil
 }
 
+// domainStatusOverride returns a result the status endpoint should serve
+// without polling the provider (ok=true), or ok=false when a real poll should
+// run and be persisted. The manual provider can't verify an external domain
+// remotely — its poll always reports "pending" — so a stored "live" there is an
+// operator confirmation (via mark-live) that the poll cannot contradict.
+// Persisting the poll result would silently revert that confirmation and put
+// the host back on the pending-domain path. Railway liveness is driven by the
+// real cert status, so no override applies there.
+func domainStatusOverride(site *core.Record) (DomainResult, bool) {
+	if getDomainProvider().Name() == "manual" && site.GetString("domain_status") == DomainStatusLive {
+		return DomainResult{Status: DomainStatusLive}, true
+	}
+	return DomainResult{}, false
+}
+
 // domainErrorMax mirrors the domain_error TextField Max in the migration.
 // applyDomainResult truncates to it so a verbose provider error can never fail
 // the save (which would turn a transient error into a permanent status-refresh

internal/domain_provider.go

@@ -68,42 +68,28 @@ type manualProvider struct{}
 
 func (manualProvider) Name() string { return "manual" }
 
-// manualRoutingRecord is the single CNAME guidance a manual (self-hosted)
-// operator must create. Both AttachDomain and DomainStatus return it so the
-// records never disappear from the UI while the domain is reported live — the
-// operator still needs to see what to point at their reverse proxy.
-func manualRoutingRecord(host string) DNSRecord {
-	return DNSRecord{
-		Type:    "CNAME",
-		Host:    host,
-		Value:   "(point this at your Primo server)",
-		Status:  "pending",
-		Purpose: "routing",
-	}
-}
-
 func (manualProvider) AttachDomain(host string) (DomainResult, error) {
 	// A base-domain subdomain is already covered by the wildcard cert/routing —
 	// nothing for the user to do.
 	if isSubdomainOfBase(host) {
 		return DomainResult{Status: DomainStatusLive}, nil
 	}
-	return DomainResult{
-		Status:  DomainStatusVerifying,
-		Records: []DNSRecord{manualRoutingRecord(host)},
-	}, nil
+	// An external domain on a self-hosted box: Primo can't talk to a platform
+	// API to issue a cert or generate correct routing records (an apex can't
+	// even use a CNAME), so we can't honestly hand over DNS records. Report
+	// "pending" and let the operator point DNS + TLS out of band, then mark the
+	// domain connected via the mark-live endpoint. No records — the UI shows a
+	// plain "point this domain at your server" instruction instead.
+	return DomainResult{Status: DomainStatusPending}, nil
 }
 
 func (manualProvider) DomainStatus(_ string, host string) (DomainResult, error) {
 	if isSubdomainOfBase(host) {
 		return DomainResult{Status: DomainStatusLive}, nil
 	}
-	// Manual providers can't verify remotely; treat as live once assigned so the
-	// UI doesn't spin forever (the operator confirms reachability out of band).
-	// Keep returning the routing record so the CNAME guidance isn't erased —
-	// applyDomainResult would otherwise overwrite the stored records with [].
-	return DomainResult{
-		Status:  DomainStatusLive,
-		Records: []DNSRecord{manualRoutingRecord(host)},
-	}, nil
+	// Manual providers can't verify remotely. Report "pending" and leave the
+	// domain there until the operator explicitly marks it connected (mark-live).
+	// Previously this optimistically returned "live" after one poll, which made
+	// the UI claim a domain was serving before any DNS was pointed at the box.
+	return DomainResult{Status: DomainStatusPending}, nil
 }

internal/domain_test.go

@@ -55,22 +55,65 @@ func TestApplyDomainResultTruncatesError(t *testing.T) {
 	}
 }
 
-// TestManualProviderStatusKeepsRecords guards that a manual-provider status
-// check keeps the routing record (so the CNAME guidance isn't erased) even
-// though it reports the domain live.
-func TestManualProviderStatusKeepsRecords(t *testing.T) {
+// TestManualProviderCustomDomainStaysPending guards that a manual-provider
+// external domain stays "pending" (not auto-flipped to live) and returns no
+// records — the operator points DNS + TLS out of band and confirms via the
+// mark-live endpoint. Previously this optimistically reported the domain live
+// after one status poll, claiming a site was serving before any DNS existed.
+func TestManualProviderCustomDomainStaysPending(t *testing.T) {
 	t.Setenv("PRIMO_BASE_DOMAIN", "acme.primo.page")
 	p := manualProvider{}
 
 	res, err := p.DomainStatus("", "theirbrand.com")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res.Status != DomainStatusLive {
-		t.Errorf("status = %q, want live", res.Status)
+	if res.Status != DomainStatusPending {
+		t.Errorf("status = %q, want pending", res.Status)
 	}
-	if len(res.Records) != 1 || res.Records[0].Type != "CNAME" {
-		t.Errorf("expected the routing CNAME to survive, got %+v", res.Records)
+	if len(res.Records) != 0 {
+		t.Errorf("expected no records for a manual custom domain, got %+v", res.Records)
+	}
+}
+
+// TestManualMarkLiveSurvivesStatusCheck guards the mark-live → status-check
+// sequence: the manual provider's poll always reports "pending" for an
+// external host, so persisting that poll would silently revert the operator's
+// explicit confirmation. The status endpoint must serve the stored "live"
+// instead of polling.
+func TestManualMarkLiveSurvivesStatusCheck(t *testing.T) {
+	t.Setenv("PRIMO_DOMAIN_PROVIDER", "")
+	t.Setenv("PRIMO_BASE_DOMAIN", "acme.primo.page")
+	app := newImportTestApp(t)
+	defer app.ResetBootstrapState()
+	site := createImportTestSite(t, app)
+
+	// Attach an external domain: pending, and status checks still poll.
+	if err := applyDomainResult(app, site, "theirbrand.com", DomainResult{Status: DomainStatusPending}); err != nil {
+		t.Fatalf("attach: %v", err)
+	}
+	if _, ok := domainStatusOverride(site); ok {
+		t.Fatal("pending manual domain should still poll the provider")
+	}
+
+	// Operator confirms reachability (mark-live persists "live").
+	if err := applyDomainResult(app, site, "theirbrand.com", DomainResult{Status: DomainStatusLive}); err != nil {
+		t.Fatalf("mark-live: %v", err)
+	}
+
+	// A later status check must serve the stored confirmation, not the poll.
+	result, ok := domainStatusOverride(site)
+	if !ok {
+		t.Fatal("manual live domain should serve the stored status, not poll")
+	}
+	if result.Status != DomainStatusLive {
+		t.Errorf("status = %q, want live", result.Status)
+	}
+
+	// Railway liveness is driven by the real cert status — never overridden.
+	t.Setenv("PRIMO_DOMAIN_PROVIDER", "railway")
+	if _, ok := domainStatusOverride(site); ok {
+		t.Error("railway provider must poll real status even when stored live")
 	}
 }
 
@@ -294,18 +337,20 @@ func TestManualProviderSubdomainShortCircuit(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if custom.Status != DomainStatusVerifying || len(custom.Records) != 1 {
-		t.Errorf("custom domain should return one record, got %+v", custom)
+	// A manual external domain can't be verified or cert-issued by Primo, so it
+	// stays pending with no records until the operator marks it connected.
+	if custom.Status != DomainStatusPending || len(custom.Records) != 0 {
+		t.Errorf("custom domain should be pending with no records, got %+v", custom)
 	}
 }
 
 func TestHostPattern(t *testing.T) {
 	// validHost mirrors the handler's check: pattern AND length limits.
 	validHost := func(h string) bool { return hostPattern.MatchString(h) && validHostLength(h) }
 
-	longLabel := strings.Repeat("a", 64) + ".com"          // one label > 63
-	longHost := strings.Repeat("a.", 130) + "com"          // total > 253
-	okLongLabel := strings.Repeat("a", 63) + ".com"        // label exactly 63
+	longLabel := strings.Repeat("a", 64) + ".com"   // one label > 63
+	longHost := strings.Repeat("a.", 130) + "com"   // total > 253
+	okLongLabel := strings.Repeat("a", 63) + ".com" // label exactly 63
 
 	valid := []string{"example.com", "sub.example.com", "a.b.c.example.com", "my-site.example.io", okLongLabel}
 	invalid := []string{