preflight checks the four adopter-set values and names their section

.github/workflows/simplycubed.yml

@@ -100,7 +100,9 @@ jobs:
         env:
           SIMPLYCUBED_AZURE_OPENAI_ENDPOINT: ${{ inputs['azure-openai-endpoint'] }}
           SIMPLYCUBED_AZURE_OPENAI_API_KEY: ${{ secrets['azure-openai-api-key'] }}
-        run: simplycubed preflight --repo-dir .
+          SIMPLYCUBED_GH_APP_CLIENT_ID: ${{ inputs['github-app-client-id'] }}
+          SIMPLYCUBED_GH_APP_PRIVATE_KEY: ${{ secrets['github-app-private-key'] }}
+        run: simplycubed preflight --repo-dir . --actions
 
       - name: Let the engine sandbox start
         # The engine sandboxes itself with a vendored bubblewrap, and bubblewrap
@@ -188,7 +190,9 @@ jobs:
         env:
           SIMPLYCUBED_AZURE_OPENAI_ENDPOINT: ${{ inputs['azure-openai-endpoint'] }}
           SIMPLYCUBED_AZURE_OPENAI_API_KEY: ${{ secrets['azure-openai-api-key'] }}
-        run: simplycubed preflight --repo-dir .
+          SIMPLYCUBED_GH_APP_CLIENT_ID: ${{ inputs['github-app-client-id'] }}
+          SIMPLYCUBED_GH_APP_PRIVATE_KEY: ${{ secrets['github-app-private-key'] }}
+        run: simplycubed preflight --repo-dir . --actions
 
       - name: Let the engine sandbox start
         # The engine sandboxes itself with a vendored bubblewrap, and bubblewrap

cmd/simplycubed/main.go

@@ -11,6 +11,7 @@ import (
 	"context"
 	_ "embed"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -52,6 +53,12 @@ func dispatch(args []string, stdout, stderr io.Writer) int {
 	}
 	fail := func(err error) int {
 		fmt.Fprintln(stderr, "error:", err)
+		// A missing adopter-set value is something to go and set, not a bug to
+		// report. Its own exit code lets a caller tell those apart without
+		// matching on message text.
+		if errors.Is(err, ErrConfigMissing) {
+			return 3
+		}
 		return 1
 	}
 	switch args[0] {
@@ -149,19 +156,19 @@ func engineEnv(cfg *config.Config) (string, error) {
 	if cfg != nil && cfg.Engine == "claude" {
 		return "", nil
 	}
-	endpoint := strings.TrimRight(os.Getenv("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT"), "/")
-	if endpoint == "" {
-		return "", fmt.Errorf("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT is not set. It is a repository variable on your own repository; a reusable workflow never inherits variables from SimplyCubed")
+	if err := requireSet("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT", sectionVariable); err != nil {
+		return "", err
 	}
+	endpoint := strings.TrimRight(os.Getenv("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT"), "/")
 	u, err := url.Parse(endpoint)
 	if err != nil {
 		return "", fmt.Errorf("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT is not a valid URL: %w", err)
 	}
 	if u.Scheme != "https" || u.Host == "" {
 		return "", fmt.Errorf("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT must be an https URL like https://<resource>.openai.azure.com, got %q", endpoint)
 	}
-	if os.Getenv("SIMPLYCUBED_AZURE_OPENAI_API_KEY") == "" {
-		return "", fmt.Errorf("SIMPLYCUBED_AZURE_OPENAI_API_KEY is not set. It is a repository secret on your own repository; a reusable workflow never inherits secrets from SimplyCubed")
+	if err := requireSet("SIMPLYCUBED_AZURE_OPENAI_API_KEY", sectionSecret); err != nil {
+		return "", err
 	}
 	return endpoint, nil
 }
@@ -171,6 +178,33 @@ func engineEnv(cfg *config.Config) (string, error) {
 // identity unknown" after the change is already made and the gate has passed.
 // The identity is the credential's own login, so commits are attributable to
 // whoever the run authenticated as.
+// ErrConfigMissing marks a missing adopter-set value, as opposed to an internal
+// failure. The two want different responses: one is something to go and set,
+// the other is a bug. dispatch turns this into its own exit code so a caller
+// can tell them apart without parsing text.
+var ErrConfigMissing = errors.New("configuration missing")
+
+// section is where a value lives in the adopter's repository settings. GitHub
+// puts Variables and Secrets on different tabs, and a value filed under the
+// wrong one reads back as empty rather than failing, so every message about a
+// missing value has to say which tab it belongs on.
+type section string
+
+const (
+	sectionVariable section = "variable"
+	sectionSecret   section = "secret"
+)
+
+// requireSet returns an error naming the value and its section when unset or
+// empty. Empty matters as much as unset: an empty string is exactly what a
+// value filed under the wrong tab looks like from here.
+func requireSet(name string, where section) error {
+	if strings.TrimSpace(os.Getenv(name)) != "" {
+		return nil
+	}
+	return fmt.Errorf("%w: %s is not set. It is a repository %s on your own repository, under Settings > Secrets and variables > Actions; a reusable workflow never inherits %ss from SimplyCubed", ErrConfigMissing, name, where, where)
+}
+
 func newVCS(self string) *vcsgit.Git {
 	if self == "" {
 		return &vcsgit.Git{}
@@ -376,6 +410,7 @@ func reply(argv []string, body string, stdout io.Writer) error {
 func preflightCmd(argv []string, stdout io.Writer) error {
 	fs := flag.NewFlagSet("preflight", flag.ContinueOnError)
 	repoDir := fs.String("repo-dir", ".", "path to the target repo checkout")
+	actions := fs.Bool("actions", false, "also check the values only a caller workflow supplies")
 	if _, err := parseInterleaved(fs, argv); err != nil {
 		return err
 	}
@@ -386,6 +421,22 @@ func preflightCmd(argv []string, stdout io.Writer) error {
 	if _, err := engineEnv(cfg); err != nil {
 		return err
 	}
+	// The App credentials are only reachable when the caller workflow exports
+	// them. A local run authenticates as the operator and never uses them, so
+	// checking them there would fail a working setup.
+	if *actions {
+		for _, v := range []struct {
+			name  string
+			where section
+		}{
+			{"SIMPLYCUBED_GH_APP_CLIENT_ID", sectionVariable},
+			{"SIMPLYCUBED_GH_APP_PRIVATE_KEY", sectionSecret},
+		} {
+			if err := requireSet(v.name, v.where); err != nil {
+				return err
+			}
+		}
+	}
 	fmt.Fprintln(stdout, "preflight ok: config and engine settings are present")
 	return nil
 }

cmd/simplycubed/main_test.go

@@ -1194,3 +1194,69 @@ func TestWorkflowRestrictedPushCoversAnyBotIdentity(t *testing.T) {
 		}
 	})
 }
+
+// Two of the four adopter-set values live under Variables and two under
+// Secrets. A value filed on the wrong tab reads back empty rather than
+// erroring, so preflight has to catch it and say which tab it belongs on.
+func TestPreflightChecksTheAdopterSetValues(t *testing.T) {
+	setAll := func(t *testing.T) {
+		t.Helper()
+		t.Setenv("SIMPLYCUBED_AZURE_OPENAI_ENDPOINT", "https://r.openai.azure.com")
+		t.Setenv("SIMPLYCUBED_AZURE_OPENAI_API_KEY", "k")
+		t.Setenv("SIMPLYCUBED_GH_APP_CLIENT_ID", "Iv23example")
+		t.Setenv("SIMPLYCUBED_GH_APP_PRIVATE_KEY", "-----BEGIN...")
+	}
+
+	for _, tc := range []struct {
+		name, unset, wantSection string
+	}{
+		{"endpoint", "SIMPLYCUBED_AZURE_OPENAI_ENDPOINT", "repository variable"},
+		{"api key", "SIMPLYCUBED_AZURE_OPENAI_API_KEY", "repository secret"},
+		{"client id", "SIMPLYCUBED_GH_APP_CLIENT_ID", "repository variable"},
+		{"private key", "SIMPLYCUBED_GH_APP_PRIVATE_KEY", "repository secret"},
+	} {
+		t.Run(tc.name+" missing names its section", func(t *testing.T) {
+			setAll(t)
+			t.Setenv(tc.unset, "")
+			err := preflightCmd([]string{"--repo-dir", repoWithConfig(t), "--actions"}, io.Discard)
+			if err == nil {
+				t.Fatalf("%s unset must fail preflight", tc.unset)
+			}
+			if !errors.Is(err, ErrConfigMissing) {
+				t.Fatalf("err = %v, want it to classify as a configuration miss", err)
+			}
+			for _, want := range []string{tc.unset, tc.wantSection} {
+				if !strings.Contains(err.Error(), want) {
+					t.Fatalf("err = %q, expected it to mention %q", err, want)
+				}
+			}
+		})
+	}
+
+	t.Run("a value filed on the wrong tab looks empty and fails the same way", func(t *testing.T) {
+		setAll(t)
+		t.Setenv("SIMPLYCUBED_GH_APP_CLIENT_ID", "   ")
+		err := preflightCmd([]string{"--repo-dir", repoWithConfig(t), "--actions"}, io.Discard)
+		if !errors.Is(err, ErrConfigMissing) {
+			t.Fatalf("err = %v, want a whitespace-only value treated as unset", err)
+		}
+	})
+
+	t.Run("a local run does not require the App pair", func(t *testing.T) {
+		setAll(t)
+		t.Setenv("SIMPLYCUBED_GH_APP_CLIENT_ID", "")
+		t.Setenv("SIMPLYCUBED_GH_APP_PRIVATE_KEY", "")
+		if err := preflightCmd([]string{"--repo-dir", repoWithConfig(t)}, io.Discard); err != nil {
+			t.Fatalf("a local run authenticates as the operator and never uses the App: %v", err)
+		}
+	})
+
+	t.Run("a configuration miss exits 3, not 1", func(t *testing.T) {
+		setAll(t)
+		t.Setenv("SIMPLYCUBED_AZURE_OPENAI_API_KEY", "")
+		var out, errOut bytes.Buffer
+		if code := dispatch([]string{"preflight", "--repo-dir", repoWithConfig(t)}, &out, &errOut); code != 3 {
+			t.Fatalf("exit = %d, want 3 so a caller can tell a missing value from a bug", code)
+		}
+	})
+}

docs/setup.md

@@ -258,6 +258,21 @@ That validates the repository config and the engine settings and exits. It is
 what the workflow runs before installing the rest of the toolchain, so a
 misconfigured repository finds out in seconds.
 
+It names the section a missing value belongs in, because Variables and Secrets
+are different tabs and a value filed under the wrong one reads back as empty
+rather than failing:
+
+```text
+error: configuration missing: SIMPLYCUBED_AZURE_OPENAI_ENDPOINT is not set. It is a
+repository variable on your own repository, under Settings > Secrets and variables >
+Actions; a reusable workflow never inherits variables from SimplyCubed
+```
+
+A missing value exits **3**, so a caller can tell "go and set this" apart from a
+bug without matching on message text. In Actions the workflow passes `--actions`,
+which also checks the App credentials; a local run authenticates as you and never
+uses them.
+
 ```sh
 simplycubed run owner/repo#N --dry-run
 ```