3 files changed, 107 insertions(+), 79 deletions(-)

M cmd/dolt-git-hook/main.go
M cmd/doltsrht/main.go
M cmd/doltsrht/main_test.go

@@ 34,6 34,7 @@ import (
 	"sourcecraft.dev/bigbes/sr-ht-core/config"
 	"sourcecraft.dev/bigbes/sr-ht-core/crypto"
 
+	"sourcecraft.dev/bigbes/sr-ht-ecore/instconf"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/internalauth"
 
 	"sourcecraft.dev/bigbes/sr-ht-dolt/core"


@@ 44,6 45,9 @@ import (
 // to this binary. Overridable via env for testing.
 const defaultDelegate = "/usr/bin/git.sr.ht-update-hook"
 
+// doltService is the config section of the service this hook provisions into.
+const doltService = "dolt.sr.ht"
+
 // provisionTimeout bounds the internal create call so a slow or down dolt.sr.ht
 // never adds more than this to a push.
 const provisionTimeout = 5 * time.Second


@@ 125,21 129,23 @@ func provisionDolt() {
 
 	// crypto.InitCrypto log.Fatalf's (os.Exit) on a missing key, which recover
 	// cannot catch — check the keys ourselves first so a misconfigured instance
-	// degrades to a skipped companion, never a hard-exiting hook.
-	if _, ok := conf.Get("sr.ht", "network-key"); !ok {
-		fmt.Fprintln(os.Stderr, "dolt-git-hook: [sr.ht]network-key not set; skipping companion provisioning")
-		return
-	}
-	if _, ok := conf.Get("webhooks", "private-key"); !ok {
-		fmt.Fprintln(os.Stderr, "dolt-git-hook: [webhooks]private-key not set; skipping companion provisioning")
+	// degrades to a skipped companion, never a hard-exiting hook. Everything
+	// this call needs is asked for at once, so an operator whose config is
+	// incomplete reads the whole list off one push instead of one key per push.
+	if err := instconf.Require(conf,
+		instconf.Need("sr.ht", "network-key"),
+		instconf.Need("webhooks", "private-key"),
+		instconf.NeedAny(doltService, "internal-origin", "origin"),
+	); err != nil {
+		fmt.Fprintf(os.Stderr, "dolt-git-hook: %v; skipping companion provisioning\n", err)
 		return
 	}
 
-	origin, ok := conf.Get("dolt.sr.ht", "internal-origin")
-	if !ok || origin == "" {
-		fmt.Fprintln(os.Stderr, "dolt-git-hook: [dolt.sr.ht]internal-origin not set; skipping companion provisioning")
-		return
-	}
+	// The internal origin when the instance has one, the external origin when it
+	// does not. An instance that routes service-to-service traffic over a private
+	// network sets the first; one that has only the public address is not
+	// misconfigured, and used to be refused here for it.
+	origin := instconf.InternalOrigin(conf, doltService)
 
 	crypto.InitCrypto(conf)
 


@@ 175,7 181,7 @@ func createCompanion(out io.Writer, origin string, pc pushContext) {
 	}
 
 	req, err := http.NewRequest("POST",
-		strings.TrimRight(origin, "/")+"/internal/repos", bytes.NewReader(body))
+		instconf.CanonicalOrigin(origin)+"/internal/repos", bytes.NewReader(body))
 	if err != nil {
 		return
 	}

@@ 22,7 22,6 @@ import (
 	"database/sql"
 	"fmt"
 	"log/slog"
-	"net/url"
 	"os"
 
 	"github.com/go-chi/chi/v5"


@@ 38,6 37,8 @@ import (
 	"sourcecraft.dev/bigbes/sr-ht-core/database"
 	"sourcecraft.dev/bigbes/sr-ht-core/server"
 
+	"sourcecraft.dev/bigbes/sr-ht-ecore/instconf"
+
 	"sourcecraft.dev/bigbes/sr-ht-dolt/authn"
 	"sourcecraft.dev/bigbes/sr-ht-dolt/remoteapi"
 	"sourcecraft.dev/bigbes/sr-ht-dolt/storage"


@@ 94,26 95,36 @@ type settings struct {
 
 // resolveSettings reads the [dolt.sr.ht] section, applying defaults and failing
 // on the keys that have no sensible default (connection-string and origin).
+//
+// Both gaps are reported together rather than one per boot. An operator filling
+// in a fresh config.ini wants the whole list in front of them, not one key per
+// restart, which is what instconf.Require is for.
 func resolveSettings(conf ini.File) (settings, error) {
-	connString, ok := conf.Get(serviceName, "connection-string")
-	if !ok || connString == "" {
+	if err := instconf.Require(conf,
+		instconf.Need(serviceName, "connection-string"),
+		instconf.Need(serviceName, "origin"),
+	); err != nil {
 		// A hint rather than a longer sentence: scribe prints it on its own
-		// line, and what an operator meeting this needs is the key to add, not
+		// line, and what an operator meeting this needs is the keys to add, not
 		// a restatement of the failure.
-		return settings{}, culpa.WithHint(
-			culpa.New("no connection-string is configured"),
-			"set [dolt.sr.ht]connection-string in config.ini")
+		return settings{}, culpa.WithHint(culpa.Wrap(err, "reading the config"),
+			"origin is what places this service in every other service's nav")
 	}
 
-	host, err := hostFromOrigin(config.GetOrigin(conf, serviceName, true))
-	if err != nil {
+	// The authority and not the bare host: the port is part of what identifies
+	// this endpoint, and https://x:8443 and https://x:9443 are two different
+	// sealed-URL hosts and two different JWT audiences. "" here means the origin
+	// is set but names no host — a scheme-less "dolt.example.org" is a path, not
+	// a URL — which is a configuration error and not a reason to guess.
+	host := instconf.OriginAuthority(instconf.ExternalOrigin(conf, serviceName))
+	if host == "" {
 		return settings{}, culpa.WithHint(
-			culpa.Wrapf(err, "[%s]origin", serviceName),
-			"origin must be protocol://host; it is what places this service in every other service's nav")
+			culpa.New(fmt.Sprintf("[%s]origin names no host", serviceName)),
+			"origin must be protocol://host, e.g. https://dolt.example.org")
 	}
 
 	return settings{
-		connString:     connString,
+		connString:     config.GetString(conf, serviceName, "connection-string", ""),
 		reposRoot:      config.GetString(conf, serviceName, "repos", defaultReposRoot),
 		staticDir:      config.GetString(conf, serviceName, "static-dir", defaultStaticDir),
 		remotesapiAddr: config.GetString(conf, serviceName, "remotesapi-listen", defaultRemotesapiAddr),


@@ 122,25 133,6 @@ func resolveSettings(conf ini.File) (settings, error) {
 	}, nil
 }
 
-// hostFromOrigin extracts the host[:port] authority from a service origin URL
-// (e.g. "https://dolt.srht.bigb.es" -> "dolt.srht.bigb.es"). A missing or
-// host-less origin is an error: the origin is required (it is what places the
-// service in every other service's nav) and drives sealed-URL and JWT-audience
-// correctness.
-func hostFromOrigin(origin string) (string, error) {
-	if origin == "" {
-		return "", fmt.Errorf("empty origin; set origin=https://<host> in config.ini")
-	}
-	u, err := url.Parse(origin)
-	if err != nil {
-		return "", fmt.Errorf("parse origin %q: %w", origin, err)
-	}
-	if u.Host == "" {
-		return "", fmt.Errorf("origin %q has no host", origin)
-	}
-	return u.Host, nil
-}
-
 func main() {
 	conf := config.LoadConfig()
 


@@ 193,6 185,20 @@ func main() {
 
 	stores := &storeManager{cache: rsrv.Cache()}
 
+	// The git-description mirror is wired only on an instance that has a
+	// git.sr.ht to ask. web.Config documents a nil Git as "no mirroring", but
+	// nothing used to produce one: core-go's client.Do walks the API-origin
+	// ladder through config.GetAPI, which panics when it reaches the end, so an
+	// instance without git.sr.ht met that as a stack trace on the first push
+	// rather than as a description it simply did not copy.
+	var git web.GitDescriber
+	if _, ok := instconf.InternalAPIOrigin(conf, "git.sr.ht"); ok {
+		git = web.GitDescriptionResolver{}
+	} else {
+		slog.Warn("no git.sr.ht API origin is configured; companion databases will not mirror their git twin's description",
+			"component", "web", "keys", instconf.APIOriginKeys())
+	}
+
 	srv.AnonRouter().Group(func(r chi.Router) {
 		r.Use(chimw.RealIP, chimw.Recoverer)
 		r.Use(config.Middleware(conf, serviceName), database.Middleware(db))


@@ 206,7 212,7 @@ func main() {
 			Repos:     web.DBAdapter{},
 			Browse:    web.BrowseAdapter{},
 			Users:     web.MetaUserResolver{},
-			Git:       web.GitDescriptionResolver{},
+			Git:       git,
 			RepoDiskPath: func(owner, name string) string {
 				return storage.RepoDiskPath(cfg.reposRoot, owner, name)
 			},

@@ 4,7 4,11 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/vaughan0/go-ini"
+
+	"sourcecraft.dev/bigbes/sr-ht-ecore/instconf"
 )
 
 // loadConf builds an ini.File directly from a literal, bypassing


@@ 88,9 92,10 @@ func TestResolveSettingsMissingConnString(t *testing.T) {
 origin=https://dolt.example.org
 `)
 
-	if _, err := resolveSettings(conf); err == nil {
-		t.Fatal("expected error for missing connection-string, got nil")
-	}
+	_, err := resolveSettings(conf)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, instconf.ErrIncompleteConfig)
+	assert.Contains(t, err.Error(), "connection-string")
 }
 
 func TestResolveSettingsMissingOrigin(t *testing.T) {


@@ 98,37 103,48 @@ func TestResolveSettingsMissingOrigin(t *testing.T) {
 connection-string=postgres://u@localhost/d
 `)
 
-	if _, err := resolveSettings(conf); err == nil {
-		t.Fatal("expected error for missing origin, got nil")
-	}
+	_, err := resolveSettings(conf)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, instconf.ErrIncompleteConfig)
+	assert.Contains(t, err.Error(), "origin")
 }
 
-func TestHostFromOrigin(t *testing.T) {
-	cases := []struct {
-		origin  string
-		want    string
-		wantErr bool
-	}{
-		{"https://dolt.srht.bigb.es", "dolt.srht.bigb.es", false},
-		{"https://dolt.srht.bigb.es:443", "dolt.srht.bigb.es:443", false},
-		{"http://127.0.0.1:5307", "127.0.0.1:5307", false},
-		{"", "", true},
-		{"not-a-url-with-no-host", "", true},
-	}
-	for _, c := range cases {
-		got, err := hostFromOrigin(c.origin)
-		if c.wantErr {
-			if err == nil {
-				t.Errorf("hostFromOrigin(%q): expected error", c.origin)
-			}
-			continue
-		}
-		if err != nil {
-			t.Errorf("hostFromOrigin(%q): %v", c.origin, err)
-			continue
-		}
-		if got != c.want {
-			t.Errorf("hostFromOrigin(%q) = %q, want %q", c.origin, got, c.want)
-		}
-	}
+// TestResolveSettingsReportsEveryMissingKey is the reason the two checks became
+// one Require: an operator with an empty section gets both keys off one boot
+// rather than one key per restart.
+func TestResolveSettingsReportsEveryMissingKey(t *testing.T) {
+	_, err := resolveSettings(loadConf(t, "[dolt.sr.ht]\n"))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "connection-string")
+	assert.Contains(t, err.Error(), "origin")
+}
+
+// TestResolveSettingsHostlessOrigin covers the gap Require cannot: a present,
+// non-blank origin that names no host. "dolt.example.org" without a scheme is a
+// URL path, and a service whose sealed chunk URLs and JWT audience are built
+// from an empty authority must refuse to start rather than guess a host.
+func TestResolveSettingsHostlessOrigin(t *testing.T) {
+	conf := loadConf(t, `[dolt.sr.ht]
+connection-string=postgres://u@localhost/d
+origin=dolt.example.org
+`)
+
+	_, err := resolveSettings(conf)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "names no host")
+}
+
+// TestResolveSettingsTrimsTheOrigin: an origin written with a trailing slash is
+// the same origin. It used to reach url.Parse untouched, which happened to
+// answer the same host; the authority now comes off the canonical form, so the
+// two spellings cannot diverge for any other consumer either.
+func TestResolveSettingsTrimsTheOrigin(t *testing.T) {
+	conf := loadConf(t, `[dolt.sr.ht]
+connection-string=postgres://u@localhost/d
+origin=https://dolt.example.org:8443/
+`)
+
+	got, err := resolveSettings(conf)
+	require.NoError(t, err)
+	assert.Equal(t, "dolt.example.org:8443", got.httpHost)
 }