From 2ec8d7201eda2b6b148fcb7b71abbbe775ce384d Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 2 Sep 2026 21:29:50 -0400 Subject: [PATCH 1/2] feat(compass-app): revive embedded dual-mode + supervisor pipeline (RIG-3197, RIG-3198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverse the RIG-2554 client-only retirement: embedded app mode returns alongside the surviving client mode, per the frozen embedded-revival design (docs/designs/ui/compass-native-embedded-revival/design.md, §A1/§A2/§A3/§A6 shell half). T-1 and T-2 land together because T-1 alone would flip the absent-app.toml default to a ModeEmbedded whose launch arm is only a transient error, and T-2 alone would not compile (embedded.go consumes T-1's appconfig API). T-1 (RIG-3197) revives dual-mode in go/internal/appconfig: ModeEmbedded is added after ModeClient so ModeClient keeps the zero value; Parse resolves absent/empty/embedded mode to ModeEmbedded (the zero-config onboarding default) and rejects the client-only server_url/ca_cert fields under embedded mode legibly; Load gains the override parameter and resolves absent-file to the embedded default rather than a first-run error; the --mode/$COMPASS_APP_MODE override returns with flag>env>file>default precedence. The RIG-2554 retirement-error copy is deleted, not kept beside a working arm. T-2 (RIG-3198) revives the in-process supervisor pipeline in go/cmd/compass-app (embedded.go, lifecycle.go, preflight_adapters.go) and the go/internal/preflight package, reconciled against current main rather than restored verbatim. The app supervises the stack through the compass-stack binary (fire-and-return): preflight then compass-stack up then a WhoAmI dial over the stack's Unix socket. Reconciliations: the app-side DSN duplicate and the DB preflight probe are dropped (postgres is now a DL-260 container the stack starts; up-Ready is the DB verification); stackUpArgs passes only up --state-dir --image --socket and none of the CLI-defaulted postgres/collector/listen flags; the revived files carry the current (linux && gtk4) || darwin build tags; the launch dispatch regains the two-arm switch handing bridge.NewUnixTarget(socket) plus the WhoAmI account id to the bridge service. Preflight v2 widens the OS check to linux|darwin, adds a darwin-only machine-ready seam (its adapter is a later task), drops the DB and uid checks (main's runner is uid-agnostic via the userns keep-id remap), and adds a fatal podman-version floor check that surfaces the podman 4.3+ requirement at the front door instead of deep inside a fire-and-return stack whose exit 0 hides the runner's own refusal. The bring-up timeout is raised to cover a three-image cold pull. shellStartupJS re-parameterizes on the resolved mode. Ledger-impact: none Refs RIG-1662 Co-authored-by: Matt Wilkinson --- go/cmd/compass-app/client_test.go | 12 +- go/cmd/compass-app/embedded.go | 423 ++++++++++++++ go/cmd/compass-app/embedded_path_test.go | 64 +++ go/cmd/compass-app/embedded_test.go | 679 +++++++++++++++++++++++ go/cmd/compass-app/lifecycle.go | 74 +++ go/cmd/compass-app/lifecycle_test.go | 109 ++++ go/cmd/compass-app/main.go | 183 ++++-- go/cmd/compass-app/preflight_adapters.go | 61 ++ go/internal/appconfig/appconfig.go | 133 +++-- go/internal/appconfig/appconfig_test.go | 274 ++++++--- go/internal/appconfig/doc.go | 29 +- go/internal/bridge/pump.go | 11 +- go/internal/preflight/doc.go | 19 + go/internal/preflight/preflight.go | 164 ++++++ go/internal/preflight/preflight_test.go | 281 ++++++++++ 15 files changed, 2329 insertions(+), 187 deletions(-) create mode 100644 go/cmd/compass-app/embedded.go create mode 100644 go/cmd/compass-app/embedded_path_test.go create mode 100644 go/cmd/compass-app/embedded_test.go create mode 100644 go/cmd/compass-app/lifecycle.go create mode 100644 go/cmd/compass-app/lifecycle_test.go create mode 100644 go/cmd/compass-app/preflight_adapters.go create mode 100644 go/internal/preflight/doc.go create mode 100644 go/internal/preflight/preflight.go create mode 100644 go/internal/preflight/preflight_test.go diff --git a/go/cmd/compass-app/client_test.go b/go/cmd/compass-app/client_test.go index 82f1f8b63..6bc68eabf 100644 --- a/go/cmd/compass-app/client_test.go +++ b/go/cmd/compass-app/client_test.go @@ -166,13 +166,13 @@ func pemEncodeCert(t *testing.T, cert *x509.Certificate) []byte { return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) } -// TestShellStartupJS covers the OQ-8 startup-global injection: the client mode -// token, __COMPASS_SERVER_URL__ present in client mode, and JSON-escaping of a -// hostile server URL so it cannot break out of the script. Client is now the -// only mode (embedded was retired in RIG-2554). +// TestShellStartupJS covers the OQ-8 startup-global injection for CLIENT mode: +// the client mode token, __COMPASS_SERVER_URL__ present in client mode, and +// JSON-escaping of a hostile server URL so it cannot break out of the script. +// (The embedded-mode arm of shellStartupJS is covered in embedded_test.go.) func TestShellStartupJS(t *testing.T) { t.Run("client injects mode and server url", func(t *testing.T) { - js, err := shellStartupJS("https://remote.example:8443") + js, err := shellStartupJS(appconfig.ModeClient.String(), "https://remote.example:8443") if err != nil { t.Fatalf("shellStartupJS err = %v, want nil", err) } @@ -186,7 +186,7 @@ func TestShellStartupJS(t *testing.T) { t.Run("hostile server url is JSON-escaped, not a breakout", func(t *testing.T) { hostile := `https://x/"+alert(1)+" cannot break out of the script or inject — the encoded form is -// always a valid JS string literal. -func shellStartupJS(serverURL string) (string, error) { - urlJSON, err := json.Marshal(serverURL) +// bundle. It assigns window.__COMPASS_MODE__ in both modes and, in client mode, +// window.__COMPASS_SERVER_URL__. Each value is JSON-encoded (encoding/json) so a +// hostile server URL containing quotes/backslashes/ cannot break out of +// the script or inject — the encoded form is always a valid JS string literal. +func shellStartupJS(mode, serverURL string) (string, error) { + modeJSON, err := json.Marshal(mode) if err != nil { - return "", fmt.Errorf("encoding startup server-url global: %w", err) + return "", fmt.Errorf("encoding startup mode global: %w", err) + } + js := "window.__COMPASS_MODE__=" + string(modeJSON) + ";" + if mode == appconfig.ModeClient.String() { + urlJSON, err := json.Marshal(serverURL) + if err != nil { + return "", fmt.Errorf("encoding startup server-url global: %w", err) + } + js += "window.__COMPASS_SERVER_URL__=" + string(urlJSON) + ";" } - js := `window.__COMPASS_MODE__="client";` + - "window.__COMPASS_SERVER_URL__=" + string(urlJSON) + ";" return js, nil } @@ -238,6 +319,36 @@ func resolveAssetsDir(flagValue string) string { return "dist" } +// resolveMode resolves the --mode/$COMPASS_APP_MODE override to feed +// appconfig.Load. An empty flag falls back to the env; both empty is "no +// override" (Load then uses app.toml, else the embedded default). +func resolveMode(flagValue string) string { + if flagValue != "" { + return flagValue + } + return os.Getenv("COMPASS_APP_MODE") +} + +// resolveSocket picks the stack socket to serve/dial: the --socket flag, else +// $COMPASS_SOCKET, else $XDG_RUNTIME_DIR/compass/server.sock (the server +// default, go/server/socket.go DefaultSocketPath). A relative XDG_RUNTIME_DIR is +// treated as unset, matching the server, so the fallback is deterministic. In +// embedded mode the supervised stack serves this socket (passed as --socket and +// dialed for WhoAmI); the client arm dials the remote over TLS instead and never +// touches it. +func resolveSocket(flagValue string) string { + if flagValue != "" { + return flagValue + } + if env := os.Getenv("COMPASS_SOCKET"); env != "" { + return env + } + if runtimeDir := os.Getenv("XDG_RUNTIME_DIR"); filepath.IsAbs(runtimeDir) { + return filepath.Join(runtimeDir, "compass", "server.sock") + } + return filepath.Join(os.Getenv("HOME"), ".compass", "server.sock") +} + // distDirForExecutable resolves the dist directory for a given executable path. // A macOS .app stages the binary at Contents/MacOS/compass-app and the UI dist // at Contents/Resources/dist (the macos-bundle tool, compass-distribution T3), diff --git a/go/cmd/compass-app/preflight_adapters.go b/go/cmd/compass-app/preflight_adapters.go new file mode 100644 index 000000000..dd1bff595 --- /dev/null +++ b/go/cmd/compass-app/preflight_adapters.go @@ -0,0 +1,61 @@ +//go:build (linux && gtk4) || darwin + +// The real host-preflight adapters for embedded mode: each is one genuine +// external effect the preflight core (go/internal/preflight) is inverted over — +// a rootless-podman probe, a podman-version floor probe, and an agent-image +// presence check. They are thin shells around os/exec and the runtime package, +// mirroring how go/internal/stack/adapters wires real effects behind the stack +// core seams; the pipeline's composition root (realPreflight in embedded.go) +// supplies them. +package main + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/RigelBuild/compass/go/internal/runtime" +) + +// podmanRootless probes that rootless podman is present and usable by running +// `podman info`. A nil error means it answered; a non-nil error wraps the +// captured stderr so the preflight failure copy names why podman is unusable. +func podmanRootless(ctx context.Context) error { + cmd := exec.CommandContext(ctx, "podman", "info") + if out, err := cmd.CombinedOutput(); err != nil { + if msg := strings.TrimSpace(string(out)); msg != "" { + return fmt.Errorf("podman info: %w: %s", err, msg) + } + return fmt.Errorf("podman info: %w", err) + } + return nil +} + +// podmanVersionAtLeastFloor probes that the host podman is new enough for the +// userns remap the runner depends on (podman >= 4.3). It reuses +// runtime.(*PodmanCLI).VerifyUsernsRemapSupport so the app's front-door gate and +// the runner's startup gate share one floor and one error copy ("podman N.N or +// newer is required …"). A nil error means the floor is met; a non-nil error +// carries that copy verbatim so preflight surfaces it before `compass-stack up` +// (design §A3 delta 4). +func podmanVersionAtLeastFloor(ctx context.Context) error { + return runtime.NewPodmanCLI().VerifyUsernsRemapSupport(ctx) +} + +// imagePresent probes that the agent image ref is present in the local store via +// `podman image exists ` (exit 0 = present, non-zero = absent). A +// non-nil error means the image is not available locally (the preflight core +// reports it is pulled from GHCR at first run); it wraps the captured stderr for +// context. +func imagePresent(ctx context.Context, image string) error { + //nolint:gosec // G204: image is an operator/env-resolved ref, argv is fixed. + cmd := exec.CommandContext(ctx, "podman", "image", "exists", image) + if out, err := cmd.CombinedOutput(); err != nil { + if msg := strings.TrimSpace(string(out)); msg != "" { + return fmt.Errorf("podman image exists %s: %w: %s", image, err, msg) + } + return fmt.Errorf("podman image exists %s: %w", image, err) + } + return nil +} diff --git a/go/internal/appconfig/appconfig.go b/go/internal/appconfig/appconfig.go index 4412642ee..1ff2c240d 100644 --- a/go/internal/appconfig/appconfig.go +++ b/go/internal/appconfig/appconfig.go @@ -11,31 +11,39 @@ import ( "github.com/BurntSushi/toml" ) -// Mode is the native app's operating mode. Client is the only mode — embedded -// mode was retired in RIG-2554 (the app no longer supervises a stack). Mode is -// kept as a validation-only concept: Load/Parse only ever yield ModeClient or a -// legible error, and the shell's launch dispatch keeps a default rejection arm. +// Mode is the native app's operating mode, selected by app.toml and an optional +// --mode/$COMPASS_APP_MODE override (design §A1). The app is dual-mode: it +// either supervises a local stack (embedded) or dials a remote one (client). type Mode int const ( // ModeClient connects to a remote compass-server over its authenticated // loopback/network door; it requires a ServerURL and may carry a CACert. + // It KEEPS the zero value so a client Config need not be spelled out. ModeClient Mode = iota + // ModeEmbedded is the local-supervisor onboarding mode: the app brings up + // and supervises a private stack in-process. It is the zero-config default + // an absent app.toml (and an empty/absent mode) resolves to, so a first + // launch of the installed app just works without any server_url. It is + // declared AFTER ModeClient so ModeClient retains the zero value. + ModeEmbedded ) -// modeStrClient is the canonical client mode string as written in app.toml — the -// single source of truth shared by String and Parse. +// modeStrClient is the canonical client mode string as written in app.toml and +// the --mode/$COMPASS_APP_MODE override — the single source of truth shared by +// String, Parse, and applyOverride. const modeStrClient = "client" -// modeStrEmbedded is the retired mode value. It is NOT a supported mode: it -// parses to a legible rejection (see Parse) naming the retirement, not a -// compatibility arm. +// modeStrEmbedded is the canonical embedded mode string, shared by String, +// Parse, and applyOverride. const modeStrEmbedded = "embedded" // String renders the mode as it is written in app.toml (the TOML mode value), // for logs and round-tripping. func (m Mode) String() string { switch m { + case ModeEmbedded: + return modeStrEmbedded case ModeClient: return modeStrClient default: @@ -47,19 +55,20 @@ func (m Mode) String() string { // (OS keychain, DL-109) nor the caller account id (WhoAmI RPC, DL-111); neither // lives in the config file. type Config struct { - // Mode is the resolved operating mode. Client is the only mode. + // Mode is the resolved operating mode (embedded or client). Mode Mode // ServerURL is the native-client base URL (an absolute https URL). It is - // always required (client is the only mode). + // required in client mode and empty in embedded mode. ServerURL string // CACert is an optional path to a private trust anchor (PEM) for a // native-client connection whose server presents a private-CA certificate. - // Empty means use the system roots. + // Empty means use the system roots. Client-only; empty in embedded mode. CACert string } // fileConfig is the on-disk TOML shape. It is decoded and then validated into a -// Config. +// Config; keeping it separate lets Parse distinguish an absent mode key from an +// explicit empty string only where that matters (both resolve to embedded). type fileConfig struct { Mode string `toml:"mode"` ServerURL string `toml:"server_url"` @@ -67,15 +76,13 @@ type fileConfig struct { } // Parse decodes and validates an app.toml byte slice into a Config. It performs -// no I/O. The rules (design §A3, client-only): -// - absent/empty mode or mode="client" → client mode, which REQUIRES a -// non-empty server_url that parses as an absolute https URL (ca_cert is -// optional); -// - mode="embedded" → a legible rejection naming the retirement (embedded mode -// was retired; run a headless stack with `compass-stack up` and point the -// app at it in client mode). This is an error string, NOT a compatibility -// arm (Global Constraint 5's sanctioned residue). -// - any other mode value is an error naming the one valid mode. +// no I/O. The rules (design §A1): +// - absent/empty mode or mode="embedded" → ModeEmbedded (the zero-config +// onboarding default). server_url and ca_cert are client-only fields, so a +// non-empty value under embedded mode is a legible error; +// - mode="client" requires a non-empty server_url that parses as an absolute +// https URL (ca_cert is optional); +// - any other mode value is an error naming the two valid modes. func Parse(data []byte) (Config, error) { var fc fileConfig md, err := toml.Decode(string(data), &fc) @@ -91,25 +98,40 @@ func Parse(data []byte) (Config, error) { } switch strings.TrimSpace(fc.Mode) { - case "", modeStrClient: + case "", modeStrEmbedded: + return parseEmbedded(fc) + case modeStrClient: return parseClient(fc) - case modeStrEmbedded: - return Config{}, errors.New( - `appconfig: mode="embedded" is no longer supported: embedded mode was retired (RIG-2554). ` + - "Run a headless Compass stack with `compass-stack up` on a dedicated machine and " + - `point the app at it in client mode (mode="client" with server_url = "https://host:8443")`) default: return Config{}, fmt.Errorf( - "appconfig: unknown mode %q in app.toml: the only valid mode is %q", - fc.Mode, modeStrClient) + "appconfig: unknown mode %q in app.toml: valid modes are %q and %q", + fc.Mode, modeStrEmbedded, modeStrClient) } } +// parseEmbedded validates the embedded-mode fields. Embedded supervises a local +// stack, so server_url and ca_cert are client-only and must be absent: a value +// under embedded mode is almost certainly a misfiled client config, so it is +// rejected legibly rather than silently ignored. +func parseEmbedded(fc fileConfig) (Config, error) { + if strings.TrimSpace(fc.ServerURL) != "" { + return Config{}, errors.New( + `appconfig: server_url is a client-only field and must not be set in embedded mode ` + + `(embedded supervises a local stack); use mode="client" to dial a remote server_url`) + } + if strings.TrimSpace(fc.CACert) != "" { + return Config{}, errors.New( + `appconfig: ca_cert is a client-only field and must not be set in embedded mode ` + + `(embedded supervises a local stack); use mode="client" to dial a remote server with a private CA`) + } + return Config{Mode: ModeEmbedded}, nil +} + // parseClient validates the client-mode fields. func parseClient(fc fileConfig) (Config, error) { if strings.TrimSpace(fc.ServerURL) == "" { return Config{}, errors.New( - `appconfig: server_url is required in app.toml (e.g. server_url = "https://host:8443")`) + `appconfig: mode="client" requires server_url in app.toml (e.g. server_url = "https://host:8443")`) } if err := validateServerURL(fc.ServerURL); err != nil { return Config{}, err @@ -145,37 +167,64 @@ func validateServerURL(raw string) error { return nil } -// Load resolves the app configuration from disk. +// Load resolves the app configuration from disk and applies an override. // // The config path is computed from the caller-provided paths (design §A4): // configHome/compass/app.toml when configHome is non-empty (the resolved // $XDG_CONFIG_HOME), else home/.config/compass/app.toml. The caller reads the // env; Load performs the resolution so it stays testable. // -// An absent file is a legible first-run error: client mode requires a server_url -// and there is no zero-config default any more (embedded mode was retired, -// RIG-2554). A present file is read and Parsed. -func Load(configHome, home string) (Config, error) { +// An absent file is not an error — it resolves to the ModeEmbedded zero-config +// onboarding default (first launch just works). A present file is read and +// Parsed. +// +// override is the resolved --mode/$COMPASS_APP_MODE value (empty = none). It is +// applied AFTER the file parse and wins: precedence is override (flag > env, +// resolved by the caller) > file > embedded-default (OQ-3). An override with no +// file present still works. +func Load(configHome, home, override string) (Config, error) { path, err := configPath(configHome, home) if err != nil { return Config{}, err } + cfg := Config{Mode: ModeEmbedded} // The path is the app's own config file, resolved from the caller's config // home — not attacker-controlled input. data, readErr := os.ReadFile(path) //nolint:gosec // G304: caller-resolved app config path, not user input switch { case readErr == nil: - return Parse(data) + cfg, err = Parse(data) + if err != nil { + return Config{}, err + } case errors.Is(readErr, os.ErrNotExist): - return Config{}, fmt.Errorf( - "appconfig: no app config found at %s: the Compass app is a client and needs a "+ - `server_url to connect to. Create it with mode="client" and `+ - `server_url = "https://host:8443" (the address of a headless stack started with `+ - "`compass-stack up`)", path) + // Absent file → embedded zero-config onboarding default; not an error. default: return Config{}, fmt.Errorf("appconfig: reading %s: %w", path, readErr) } + + return applyOverride(cfg, override) +} + +// applyOverride applies the resolved --mode/$COMPASS_APP_MODE override on top of +// the file-derived config. An empty override is a no-op. An override to embedded +// clears the client-only fields; an override to client keeps whatever server_url +// and ca_cert the file supplied, then re-validates them so an override into +// client mode without a usable server_url fails legibly. +func applyOverride(cfg Config, override string) (Config, error) { + switch strings.TrimSpace(override) { + case "": + return cfg, nil + case modeStrEmbedded: + return Config{Mode: ModeEmbedded}, nil + case modeStrClient: + return parseClient(fileConfig{ServerURL: cfg.ServerURL, CACert: cfg.CACert}) + default: + return Config{}, fmt.Errorf( + "appconfig: unknown mode override %q: valid modes are %q and %q", + override, modeStrEmbedded, modeStrClient) + } } // configPath computes the app.toml path from the caller-resolved config home and diff --git a/go/internal/appconfig/appconfig_test.go b/go/internal/appconfig/appconfig_test.go index eb75846cf..bbcfeb73e 100644 --- a/go/internal/appconfig/appconfig_test.go +++ b/go/internal/appconfig/appconfig_test.go @@ -7,30 +7,83 @@ import ( "testing" ) -func TestParse(t *testing.T) { - tests := []struct { - name string - data string - want Config - wantErr bool - errSubstrs []string - }{ - { - name: "empty file → client (server_url required)", +type parseCase struct { + name string + data string + want Config + wantErr bool + errSubstrs []string +} + +func runParseCases(t *testing.T, tests []parseCase) { + t.Helper() + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := Parse([]byte(tc.data)) + if tc.wantErr { + if err == nil { + t.Fatalf("want error, got config %+v", got) + } + for _, sub := range tc.errSubstrs { + if !strings.Contains(err.Error(), sub) { + t.Errorf("error %q missing substring %q", err, sub) + } + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.want { + t.Errorf("got %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestParseEmbedded(t *testing.T) { + runParseCases(t, []parseCase{ + { + name: "empty file → embedded (zero-config default)", data: "", - // Absent mode defaults to client, which requires server_url; an - // empty file therefore fails legibly rather than resolving. + want: Config{Mode: ModeEmbedded}, + }, + { + name: "explicit embedded mode", + data: `mode = "embedded"`, + want: Config{Mode: ModeEmbedded}, + }, + { + name: "whitespace-only mode → embedded", + data: `mode = " "`, + want: Config{Mode: ModeEmbedded}, + }, + { + name: "embedded with server_url → legible reject", + data: "mode = \"embedded\"\nserver_url = \"https://host:8443\"\n", wantErr: true, - errSubstrs: []string{"server_url"}, + errSubstrs: []string{"server_url", "client-only", "embedded"}, }, { - name: "client with server_url", - data: "mode = \"client\"\nserver_url = \"https://host:8443\"\n", - want: Config{Mode: ModeClient, ServerURL: "https://host:8443"}, + name: "embedded with ca_cert → legible reject", + data: "mode = \"embedded\"\nca_cert = \"/etc/anchor.pem\"\n", + wantErr: true, + errSubstrs: []string{"ca_cert", "client-only", "embedded"}, }, { - name: "absent mode with server_url → client", - data: "server_url = \"https://host:8443\"\n", + name: "absent mode with server_url → legible reject (embedded default is client-free)", + data: "server_url = \"https://host:8443\"\n", + wantErr: true, + errSubstrs: []string{"server_url", "client-only"}, + }, + }) +} + +func TestParseClient(t *testing.T) { + runParseCases(t, []parseCase{ + { + name: "client with server_url", + data: "mode = \"client\"\nserver_url = \"https://host:8443\"\n", want: Config{Mode: ModeClient, ServerURL: "https://host:8443"}, }, { @@ -38,12 +91,6 @@ func TestParse(t *testing.T) { data: "mode = \"client\"\nserver_url = \"https://host:8443\"\nca_cert = \"/etc/anchor.pem\"\n", want: Config{Mode: ModeClient, ServerURL: "https://host:8443", CACert: "/etc/anchor.pem"}, }, - { - name: "embedded → legible retirement rejection", - data: `mode = "embedded"`, - wantErr: true, - errSubstrs: []string{"embedded", "retired", "compass-stack up", "client"}, - }, { name: "client missing server_url → error", data: `mode = "client"`, @@ -62,24 +109,6 @@ func TestParse(t *testing.T) { wantErr: true, errSubstrs: []string{"server_url"}, }, - { - name: "unknown mode → error", - data: `mode = "proxy"`, - wantErr: true, - errSubstrs: []string{"proxy", "client"}, - }, - { - name: "malformed toml → error", - data: "mode = ", - wantErr: true, - errSubstrs: []string{"app.toml"}, - }, - { - name: "whitespace-only mode → client (server_url required)", - data: `mode = " "`, - wantErr: true, - errSubstrs: []string{"server_url"}, - }, { name: "client whitespace-only server_url → error", data: "mode = \"client\"\nserver_url = \" \"\n", @@ -92,57 +121,60 @@ func TestParse(t *testing.T) { wantErr: true, errSubstrs: []string{"credentials", "keychain"}, }, + { + name: "unknown mode → error naming both modes", + data: `mode = "proxy"`, + wantErr: true, + errSubstrs: []string{"proxy", "embedded", "client"}, + }, + { + name: "malformed toml → error", + data: "mode = ", + wantErr: true, + errSubstrs: []string{"app.toml"}, + }, { name: "unknown key → error", data: "mode = \"client\"\nserver_url = \"https://host:8443\"\ncacert = \"/etc/anchor.pem\"\n", wantErr: true, errSubstrs: []string{"unknown key", "cacert"}, }, - } + }) +} - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got, err := Parse([]byte(tc.data)) - if tc.wantErr { - if err == nil { - t.Fatalf("want error, got config %+v", got) - } - for _, sub := range tc.errSubstrs { - if !strings.Contains(err.Error(), sub) { - t.Errorf("error %q missing substring %q", err, sub) - } - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != tc.want { - t.Errorf("got %+v, want %+v", got, tc.want) - } - }) +// TestModeClientIsZeroValue pins the zero-value contract: ModeClient MUST be the +// zero value so an unspelled Config{} means client, never embedded. Inserting +// ModeEmbedded before ModeClient in the const block would silently flip this and +// route every zero-valued Config to embedded. +func TestModeClientIsZeroValue(t *testing.T) { + var zero Mode + if zero != ModeClient { + t.Fatalf("zero-value Mode = %v (%d), want ModeClient (0)", zero, int(zero)) + } + if int(ModeClient) != 0 { + t.Errorf("ModeClient = %d, want 0", int(ModeClient)) + } + if int(ModeEmbedded) == 0 { + t.Errorf("ModeEmbedded = 0, must not share the zero value with ModeClient") } } -// TestLoadAbsentFileIsFirstRunError: an absent app.toml is a legible first-run -// error (client mode needs a server_url; embedded's zero-config default was -// retired in RIG-2554), naming the config path and pointing at the client setup. -func TestLoadAbsentFileIsFirstRunError(t *testing.T) { +// TestLoadAbsentFileIsEmbeddedDefault: an absent app.toml is NOT an error — it +// resolves to the embedded zero-config onboarding default. +func TestLoadAbsentFileIsEmbeddedDefault(t *testing.T) { dir := t.TempDir() - got, err := Load(dir, "") - if err == nil { - t.Fatalf("absent file: want a first-run error, got config %+v", got) + got, err := Load(dir, "", "") + if err != nil { + t.Fatalf("absent file: unexpected error: %v", err) } - for _, sub := range []string{"app.toml", "server_url", "client"} { - if !strings.Contains(err.Error(), sub) { - t.Errorf("first-run error %q missing substring %q", err, sub) - } + if want := (Config{Mode: ModeEmbedded}); got != want { + t.Errorf("got %+v, want %+v", got, want) } } func TestLoadReadsPresentFile(t *testing.T) { dir := writeConfig(t, "mode = \"client\"\nserver_url = \"https://host:8443\"\n") - got, err := Load(dir, "") + got, err := Load(dir, "", "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -151,18 +183,14 @@ func TestLoadReadsPresentFile(t *testing.T) { } } -// TestLoadEmbeddedFileIsRejected: a present file selecting the retired embedded -// mode is rejected through Load (not just Parse) with the retirement copy. -func TestLoadEmbeddedFileIsRejected(t *testing.T) { +func TestLoadEmbeddedFile(t *testing.T) { dir := writeConfig(t, `mode = "embedded"`) - got, err := Load(dir, "") - if err == nil { - t.Fatalf("embedded file: want a rejection, got config %+v", got) + got, err := Load(dir, "", "") + if err != nil { + t.Fatalf("embedded file: unexpected error: %v", err) } - for _, sub := range []string{"embedded", "retired", "compass-stack up"} { - if !strings.Contains(err.Error(), sub) { - t.Errorf("embedded rejection %q missing substring %q", err, sub) - } + if want := (Config{Mode: ModeEmbedded}); got != want { + t.Errorf("got %+v, want %+v", got, want) } } @@ -175,7 +203,7 @@ func TestLoadHomeFallbackPath(t *testing.T) { if err := os.WriteFile(path, []byte("mode = \"client\"\nserver_url = \"https://host:8443\"\n"), 0o644); err != nil { t.Fatal(err) } - got, err := Load("", home) + got, err := Load("", home, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -184,10 +212,82 @@ func TestLoadHomeFallbackPath(t *testing.T) { } } +// TestLoadOverridePrecedence pins the override > file > default precedence +// (OQ-3). The override is the resolved --mode/$COMPASS_APP_MODE value; the +// caller resolves flag > env into that single string. +func TestLoadOverridePrecedence(t *testing.T) { + t.Run("override embedded wins over client file", func(t *testing.T) { + dir := writeConfig(t, "mode = \"client\"\nserver_url = \"https://host:8443\"\n") + got, err := Load(dir, "", modeStrEmbedded) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := (Config{Mode: ModeEmbedded}); got != want { + t.Errorf("got %+v, want %+v", got, want) + } + }) + + t.Run("override client with no file needs server_url", func(t *testing.T) { + dir := t.TempDir() + if _, err := Load(dir, "", modeStrClient); err == nil { + t.Fatal("override to client without a server_url: want error, got nil") + } + }) + + t.Run("override client keeps file server_url", func(t *testing.T) { + dir := writeConfig(t, "mode = \"client\"\nserver_url = \"https://host:8443\"\n") + got, err := Load(dir, "", modeStrClient) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := (Config{Mode: ModeClient, ServerURL: "https://host:8443"}); got != want { + t.Errorf("got %+v, want %+v", got, want) + } + }) + + t.Run("empty override falls through to file", func(t *testing.T) { + dir := writeConfig(t, "mode = \"client\"\nserver_url = \"https://host:8443\"\n") + got, err := Load(dir, "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := (Config{Mode: ModeClient, ServerURL: "https://host:8443"}); got != want { + t.Errorf("got %+v, want %+v", got, want) + } + }) + + t.Run("empty override with absent file → embedded default", func(t *testing.T) { + dir := t.TempDir() + got, err := Load(dir, "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := (Config{Mode: ModeEmbedded}); got != want { + t.Errorf("got %+v, want %+v", got, want) + } + }) + + t.Run("unknown override → error naming both modes", func(t *testing.T) { + dir := t.TempDir() + _, err := Load(dir, "", "proxy") + if err == nil { + t.Fatal("unknown override: want error, got nil") + } + for _, sub := range []string{"proxy", "embedded", "client"} { + if !strings.Contains(err.Error(), sub) { + t.Errorf("override error %q missing substring %q", err, sub) + } + } + }) +} + func TestModeString(t *testing.T) { if got := ModeClient.String(); got != "client" { t.Errorf("ModeClient.String() = %q, want client", got) } + if got := ModeEmbedded.String(); got != "embedded" { + t.Errorf("ModeEmbedded.String() = %q, want embedded", got) + } } // writeConfig writes app.toml under a fresh temp configHome and returns that diff --git a/go/internal/appconfig/doc.go b/go/internal/appconfig/doc.go index 1363acaac..7d45356c8 100644 --- a/go/internal/appconfig/doc.go +++ b/go/internal/appconfig/doc.go @@ -1,15 +1,24 @@ -// Package appconfig is the native app's client config parser (design §A3). One -// file — $XDG_CONFIG_HOME/compass/app.toml (fallback ~/.config/compass/app.toml) -// — configures the native-client connection: a base URL plus an optional -// private-anchor CA cert. Embedded mode was retired in RIG-2554, so client is -// the only mode; a mode="embedded" value parses to a legible rejection, not a -// compatibility arm. +// Package appconfig is the native app's config parser (design §A1). One file — +// $XDG_CONFIG_HOME/compass/app.toml (fallback ~/.config/compass/app.toml) — +// selects the app's operating mode and, in client mode, its connection. +// +// The app is dual-mode: +// - embedded (the zero-config onboarding default): the app supervises a +// private local stack in-process. An absent file, an empty mode, or +// mode="embedded" resolves here, so a first launch just works. server_url +// and ca_cert are client-only fields and must be absent in embedded mode. +// - client: the app dials a remote compass-server. It requires a server_url +// that is an absolute https URL (cleartext is refused) and may carry an +// optional ca_cert private trust anchor. +// +// A --mode/$COMPASS_APP_MODE override, resolved by the caller and passed to +// Load, wins over the file: precedence is override > file > embedded-default. // // The core is pure: Parse decodes and validates a TOML byte slice with no I/O, -// and Load layers path resolution on top. Mirroring the stack package idiom, the -// caller resolves every path — Load takes configHome and home as parameters (the -// caller reads the env), so the resolution is fully unit-testable without -// touching real $HOME/$XDG_CONFIG_HOME. +// and Load layers path resolution and the override on top. Mirroring the stack +// package idiom, the caller resolves every path — Load takes configHome and home +// as parameters (the caller reads the env), so the resolution is fully +// unit-testable without touching real $HOME/$XDG_CONFIG_HOME. // // The config file carries neither the bearer token (entered once and stored in // the OS keychain, DL-109) nor the caller account id (resolved by the WhoAmI diff --git a/go/internal/bridge/pump.go b/go/internal/bridge/pump.go index c7242f5b1..0c0060e09 100644 --- a/go/internal/bridge/pump.go +++ b/go/internal/bridge/pump.go @@ -74,12 +74,11 @@ type Call struct { // Target is a resolved daemon endpoint the pump forwards against. It holds the // HTTP client wired to reach the daemon and the base URL to build requests from. // -// Production wires a TLS/network-door target (native-client mode): a constructor -// supplies a client with a TLS-dialing transport and an https base URL, and the -// pump forwarding logic is unchanged. [NewUnixTarget] remains only as a -// test-harness h2c-over-UDS stub — its production caller was removed in T-1 -// (RIG-2554); its sole users are the pump test suite (pump_test.go) and two -// compass-app tests. +// Two production wirings build a Target. Embedded mode wires a +// cleartext-HTTP/2-over-UDS target ([NewUnixTarget]) at the private stack's Unix +// socket; native-client mode wires a TLS/network-door target +// ([NewTLSTarget]) at an https base URL. The pump forwarding logic is identical +// for both. type Target struct { client *http.Client baseURL string diff --git a/go/internal/preflight/doc.go b/go/internal/preflight/doc.go new file mode 100644 index 000000000..44a9eff5f --- /dev/null +++ b/go/internal/preflight/doc.go @@ -0,0 +1,19 @@ +// Package preflight is the native app's embedded-mode host preflight: the set of +// checks that must pass BEFORE spawning the embedded stack (compass-stack up) +// per the compass-native-embedded-revival design (§A3). Each check produces +// actionable failure copy so an operator on an unsupported host sees the precise +// precondition to fix, rather than a deep failure inside the stack. +// +// The checker core is inverted over injected effect functions (see Deps), +// mirroring the go/internal/stack idiom: every genuine external effect — +// probing rootless podman, the podman version floor, the darwin podman machine, +// and the local image store — is a func the caller supplies. The core imports +// none of those subsystems, so unit tests supply stubs and no test shells out. +// +// The checks are cross-OS: the OS check accepts linux or darwin, and a +// darwin-only machine check joins the set when its adapter is wired. The host +// uid is deliberately NOT checked — the runner is uid-agnostic now via the +// --userns=keep-id:uid= remap, so a uid gate would break launch on every host +// with uid != 1000; the importable single source of truth for the agent uid is +// go/internal/agentuid.AgentUID. +package preflight diff --git a/go/internal/preflight/preflight.go b/go/internal/preflight/preflight.go new file mode 100644 index 000000000..aec124e14 --- /dev/null +++ b/go/internal/preflight/preflight.go @@ -0,0 +1,164 @@ +package preflight + +import ( + "context" + "errors" + "fmt" + "strings" +) + +// Deps is the set of external effects the preflight core is inverted over. Each +// func field is one genuine external effect; the real adapters (which shell out +// to podman and inspect the local image store) are supplied at the wiring +// boundary, and unit tests supply stubs. The core imports none of those +// subsystems itself. +type Deps struct { + // GOOS is the host operating system (runtime.GOOS at the wiring boundary). + // Injected rather than read directly so the OS check is unit-testable. + GOOS string + // PodmanRootless probes that rootless podman is present and usable. A nil + // error means it is available; a non-nil error explains why not. + PodmanRootless func(ctx context.Context) error + // PodmanVersion probes that the host podman is new enough for the userns + // remap the runner depends on (podman >= 4.3, where --userns=keep-id:uid= + // is available). A nil error means the floor is met; a non-nil error carries + // the "podman N.N or newer is required" copy the runner would otherwise emit + // deep inside a fire-and-return stack whose exit 0 hides it. Surfaced at the + // front door instead (design §A3 delta 4). + PodmanVersion func(ctx context.Context) error + // MachineReady probes that the darwin podman machine (the Linux VM podman + // runs inside on macOS) is up. Consulted ONLY on darwin; nil on linux (there + // is no machine to check). A nil error means ready; a non-nil error explains + // why not. The darwin adapter that supplies it lands in T-6 (design §A5); a + // nil MachineReady on darwin leaves the check absent until then. + MachineReady func(ctx context.Context) error + // ImagePresent probes that the given agent image ref is present in the local + // container store. A nil error means present; a non-nil error means it is not + // available locally (it is pulled from GHCR at first run). + ImagePresent func(ctx context.Context, image string) error +} + +// Params is what the checks need from the caller: the resolved agent image ref. +type Params struct { + // AgentImage is the container image ref the embedded runner runs; its + // presence in the local store is checked. + AgentImage string +} + +// Result is the outcome of one preflight check. Name identifies the check; OK +// reports whether it passed; Detail carries the actionable failure copy when +// !OK (and may be empty when OK). +type Result struct { + Name string + OK bool + Detail string +} + +// Check names, stable for logs and error copy. +const ( + checkOS = "os" + checkPodman = "podman" + checkPodmanVersion = "podman-version" + checkMachine = "machine" + checkImage = "image" +) + +// Exported aliases of the check names, so callers can classify results by check +// (e.g. the wiring boundary hard-gates host-capability checks and treats the +// image check as advisory). These are additive: the unexported names above stay +// the values written into Result.Name, and these consts alias them so a caller's +// classification cannot drift from the Run implementation. +const ( + CheckOS = checkOS + CheckPodman = checkPodman + CheckPodmanVersion = checkPodmanVersion + CheckMachine = checkMachine + CheckImage = checkImage +) + +// Run executes every host precondition in order and returns one Result per +// check. It does NOT short-circuit: an operator should see every failing +// precondition at once, so all checks run even after an earlier failure. Call +// the returned Results' Err method to fold the failures into one legible error. +func (d Deps) Run(ctx context.Context, p Params) Results { + results := make(Results, 0, 5) + + // (1) OS is supported: linux or darwin (Windows/WSL is out of scope, OQ-4). + osRes := Result{Name: checkOS, OK: d.GOOS == "linux" || d.GOOS == "darwin"} + if !osRes.OK { + osRes.Detail = "embedded mode runs on linux or darwin, this host is " + d.GOOS + } + results = append(results, osRes) + + // (2) Rootless podman present. + podmanRes := Result{Name: checkPodman, OK: true} + if err := d.PodmanRootless(ctx); err != nil { + podmanRes.OK = false + podmanRes.Detail = fmt.Sprintf("rootless podman is required: %v", err) + } + results = append(results, podmanRes) + + // (3) Podman is new enough for the userns remap (>= 4.3). The runner + // enforces this at startup, but that refusal is swallowed on the embedded + // fire-and-return path (design §A3 delta 4), so it is surfaced here at the + // front door. The probe's error already carries the "podman N.N or newer is + // required" copy, so it is used verbatim. + pvRes := Result{Name: checkPodmanVersion, OK: true} + if err := d.PodmanVersion(ctx); err != nil { + pvRes.OK = false + pvRes.Detail = err.Error() + } + results = append(results, pvRes) + + // (4) Darwin podman machine ready. macOS runs podman inside a Linux VM; the + // check is consulted ONLY on darwin, and only when an adapter is wired (the + // darwin adapter lands in T-6). On linux there is no machine, so the check + // is absent. + if d.GOOS == "darwin" && d.MachineReady != nil { + machineRes := Result{Name: checkMachine, OK: true} + if err := d.MachineReady(ctx); err != nil { + machineRes.OK = false + machineRes.Detail = fmt.Sprintf("the podman machine is not ready: %v", err) + } + results = append(results, machineRes) + } + + // (5) Agent image present in the local store. Reporting "not available + // locally" is the correct behavior, not a stub: the image is pulled from + // GHCR at first run. + imageRes := Result{Name: checkImage, OK: true} + if err := d.ImagePresent(ctx, p.AgentImage); err != nil { + imageRes.OK = false + imageRes.Detail = fmt.Sprintf( + "agent image %s is not available locally; it is pulled from GHCR at "+ + "first run: %v", p.AgentImage, err) + } + results = append(results, imageRes) + + return results +} + +// Results is a preflight run's set of check outcomes. +type Results []Result + +// Err aggregates the failed checks into one legible multi-line error, or returns +// nil when every check passed. Returning all failures at once lets the operator +// fix every unmet precondition in a single pass. +func (rs Results) Err() error { + var failed []Result + for _, r := range rs { + if !r.OK { + failed = append(failed, r) + } + } + if len(failed) == 0 { + return nil + } + var b strings.Builder + b.WriteString("embedded-mode preflight failed:") + for _, r := range failed { + b.WriteString("\n - ") + b.WriteString(r.Detail) + } + return errors.New(b.String()) +} diff --git a/go/internal/preflight/preflight_test.go b/go/internal/preflight/preflight_test.go new file mode 100644 index 000000000..4c15aa1b9 --- /dev/null +++ b/go/internal/preflight/preflight_test.go @@ -0,0 +1,281 @@ +package preflight + +import ( + "context" + "errors" + "strings" + "testing" +) + +// okDeps returns a Deps whose every injected effect passes, for the given host +// GOOS. Tests override individual fields to drive one failure at a time. +func okDeps(goos string) Deps { + return Deps{ + GOOS: goos, + PodmanRootless: func(context.Context) error { return nil }, + PodmanVersion: func(context.Context) error { return nil }, + ImagePresent: func(context.Context, string) error { return nil }, + } +} + +var testParams = Params{ + AgentImage: "ghcr.io/rigelbuild/compass-agent:latest", +} + +// resultByName finds a check result by its Name. +func resultByName(t *testing.T, rs []Result, name string) Result { + t.Helper() + for _, r := range rs { + if r.Name == name { + return r + } + } + t.Fatalf("no result named %q in %v", name, rs) + return Result{} +} + +func TestRunAllPass(t *testing.T) { + ctx := context.Background() + rs := okDeps("linux").Run(ctx, testParams) + + // linux: os, podman, podman-version, image (no machine check on linux). + if len(rs) != 4 { + t.Fatalf("want 4 results on linux, got %d: %v", len(rs), rs) + } + for _, r := range rs { + if !r.OK { + t.Errorf("check %q failed: %s", r.Name, r.Detail) + } + } + if err := rs.Err(); err != nil { + t.Errorf("want nil error when all pass, got %v", err) + } +} + +func TestRunWrongGOOS(t *testing.T) { + ctx := context.Background() + rs := okDeps("windows").Run(ctx, testParams) + + got := resultByName(t, rs, checkOS) + if got.OK { + t.Fatal("os check should fail on windows") + } + for _, tok := range []string{"linux", "darwin", "windows"} { + if !strings.Contains(got.Detail, tok) { + t.Errorf("os detail %q missing token %q", got.Detail, tok) + } + } + assertErrContains(t, rs.Err(), "windows") +} + +// TestRunDarwinPasses: darwin is a supported OS. With a ready machine adapter +// wired, all checks pass and the machine check appears in the results. +func TestRunDarwinPasses(t *testing.T) { + ctx := context.Background() + d := okDeps("darwin") + d.MachineReady = func(context.Context) error { return nil } + rs := d.Run(ctx, testParams) + + if resultByName(t, rs, checkOS).OK != true { + t.Error("os check should pass on darwin") + } + if got := resultByName(t, rs, checkMachine); !got.OK { + t.Errorf("machine check should pass with a ready adapter: %s", got.Detail) + } + if err := rs.Err(); err != nil { + t.Errorf("want nil error on darwin all-pass, got %v", err) + } +} + +// TestRunMachineCheckAbsentOnLinux: linux has no podman machine, so the machine +// check never appears even if a MachineReady adapter is somehow wired. +func TestRunMachineCheckAbsentOnLinux(t *testing.T) { + ctx := context.Background() + d := okDeps("linux") + d.MachineReady = func(context.Context) error { return errors.New("should not be called on linux") } + rs := d.Run(ctx, testParams) + + for _, r := range rs { + if r.Name == checkMachine { + t.Fatalf("machine check present on linux: %v", rs) + } + } +} + +// TestRunMachineNotReadyOnDarwin: a darwin machine that is not ready fails the +// machine check and folds into the aggregated error. +func TestRunMachineNotReadyOnDarwin(t *testing.T) { + ctx := context.Background() + d := okDeps("darwin") + d.MachineReady = func(context.Context) error { return errors.New("machine stopped") } + rs := d.Run(ctx, testParams) + + got := resultByName(t, rs, checkMachine) + if got.OK { + t.Fatal("machine check should fail when the machine is not ready") + } + if !strings.Contains(got.Detail, "machine stopped") { + t.Errorf("machine detail %q missing probe error", got.Detail) + } + assertErrContains(t, rs.Err(), "machine stopped") +} + +// TestRunMachineAbsentOnDarwinWithoutAdapter: on darwin with no MachineReady +// adapter wired (the pre-T-6 state), the machine check is absent rather than a +// spurious failure — the seam is wired, the adapter lands in T-6. +func TestRunMachineAbsentOnDarwinWithoutAdapter(t *testing.T) { + ctx := context.Background() + rs := okDeps("darwin").Run(ctx, testParams) + + for _, r := range rs { + if r.Name == checkMachine { + t.Fatalf("machine check present on darwin without an adapter: %v", rs) + } + } + if err := rs.Err(); err != nil { + t.Errorf("want nil error on darwin with no machine adapter, got %v", err) + } +} + +func TestRunPodmanProbeFails(t *testing.T) { + ctx := context.Background() + d := okDeps("linux") + d.PodmanRootless = func(context.Context) error { + return errors.New("podman socket not found") + } + rs := d.Run(ctx, testParams) + + got := resultByName(t, rs, checkPodman) + if got.OK { + t.Fatal("podman check should fail when probe errors") + } + if !strings.Contains(got.Detail, "podman socket not found") { + t.Errorf("podman detail %q missing probe error", got.Detail) + } + assertErrContains(t, rs.Err(), "rootless podman") +} + +// TestRunPodmanVersionGate is the delta-4 gate: a below-floor podman fails FATAL +// carrying the "podman N.N or newer" copy verbatim; at/above the floor passes. +// The version probe is injected, so the test is hermetic (no real podman). The +// probe's error copy is what VerifyUsernsRemapSupport emits, reused verbatim by +// the check. +func TestRunPodmanVersionGate(t *testing.T) { + // The exact copy runtime.(*PodmanCLI).VerifyUsernsRemapSupport emits below + // the floor, which the injected probe surfaces and the check carries through. + const belowFloorCopy = "podman 4.3 or newer is required (the container userns " + + "remap --userns=keep-id:uid=,gid= is a 4.3+ option), but this host has podman 3.4.4" + + tests := []struct { + name string + probe func(context.Context) error + wantOK bool + wantToken string + }{ + { + name: "below floor is refused", + probe: func(context.Context) error { return errors.New(belowFloorCopy) }, + wantOK: false, + wantToken: "podman 4.3 or newer is required", + }, + { + name: "at or above floor passes", + probe: func(context.Context) error { return nil }, + wantOK: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + d := okDeps("linux") + d.PodmanVersion = tc.probe + rs := d.Run(ctx, testParams) + + got := resultByName(t, rs, checkPodmanVersion) + if got.OK != tc.wantOK { + t.Fatalf("podman-version check OK = %v, want %v (detail %q)", got.OK, tc.wantOK, got.Detail) + } + if tc.wantOK { + if err := rs.Err(); err != nil { + t.Errorf("want nil error at/above floor, got %v", err) + } + return + } + if !strings.Contains(got.Detail, tc.wantToken) { + t.Errorf("podman-version detail %q missing token %q", got.Detail, tc.wantToken) + } + assertErrContains(t, rs.Err(), tc.wantToken) + }) + } +} + +func TestRunImageAbsent(t *testing.T) { + ctx := context.Background() + d := okDeps("linux") + d.ImagePresent = func(context.Context, string) error { + return errors.New("no such image") + } + rs := d.Run(ctx, testParams) + + got := resultByName(t, rs, checkImage) + if got.OK { + t.Fatal("image check should fail when image absent") + } + if !strings.Contains(got.Detail, testParams.AgentImage) { + t.Errorf("image detail %q missing image ref %q", got.Detail, testParams.AgentImage) + } + assertErrContains(t, rs.Err(), testParams.AgentImage) +} + +// TestRunMultipleFailures asserts Run does not short-circuit: every failing +// precondition is reported and appears in the aggregated error. +func TestRunMultipleFailures(t *testing.T) { + ctx := context.Background() + d := Deps{ + GOOS: "windows", + PodmanRootless: func(context.Context) error { return errors.New("no podman") }, + PodmanVersion: func(context.Context) error { return errors.New("podman too old") }, + ImagePresent: func(context.Context, string) error { return errors.New("no image") }, + } + rs := d.Run(ctx, testParams) + + for _, name := range []string{checkOS, checkPodman, checkPodmanVersion, checkImage} { + if resultByName(t, rs, name).OK { + t.Errorf("check %q should have failed", name) + } + } + + err := rs.Err() + if err == nil { + t.Fatal("want aggregated error for multiple failures, got nil") + } + // Every failing check's load-bearing token must appear in one error. + for _, tok := range []string{ + "windows", "no podman", "podman too old", testParams.AgentImage, + } { + if !strings.Contains(err.Error(), tok) { + t.Errorf("aggregated error %q missing token %q", err.Error(), tok) + } + } +} + +// TestResultsErrMethod exercises the named-slice Err method directly. +func TestResultsErrMethod(t *testing.T) { + if err := (Results{{Name: checkOS, OK: true}}).Err(); err != nil { + t.Errorf("want nil for all-OK, got %v", err) + } + err := (Results{{Name: checkImage, OK: false, Detail: "boom"}}).Err() + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("want error containing detail, got %v", err) + } +} + +func assertErrContains(t *testing.T, err error, tok string) { + t.Helper() + if err == nil { + t.Fatalf("want error containing %q, got nil", tok) + } + if !strings.Contains(err.Error(), tok) { + t.Errorf("error %q missing token %q", err.Error(), tok) + } +} From 3b93f1f8568f0a6ff0f1490237a05a59ab96a8c8 Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 2 Sep 2026 22:40:41 -0400 Subject: [PATCH 2/2] test(compass-app): cover embedded shellStartupJS arm; comment fixes (RIG-3197) Review fixes on the embedded-revival T-1+T-2 slice. Add an embedded subtest to TestShellStartupJS asserting the embedded arm emits window.__COMPASS_MODE__="embedded" and omits __COMPASS_SERVER_URL__ (a non-empty server URL is ignored in embedded mode), pinning the OQ-8 startup-global contract the UI embedded boot arm reads; correct the client_test.go doc comment that falsely claimed this arm was already covered. Tighten the launch() bring-up context comment to name the actual call site. Ledger-impact: none Refs RIG-1662 Co-authored-by: Matt Wilkinson --- go/cmd/compass-app/client_test.go | 22 +++++++++++++++++++--- go/cmd/compass-app/main.go | 9 +++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/go/cmd/compass-app/client_test.go b/go/cmd/compass-app/client_test.go index 6bc68eabf..dbc4848cf 100644 --- a/go/cmd/compass-app/client_test.go +++ b/go/cmd/compass-app/client_test.go @@ -166,10 +166,9 @@ func pemEncodeCert(t *testing.T, cert *x509.Certificate) []byte { return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) } -// TestShellStartupJS covers the OQ-8 startup-global injection for CLIENT mode: -// the client mode token, __COMPASS_SERVER_URL__ present in client mode, and +// TestShellStartupJS covers the OQ-8 startup-global injection in both modes: the +// mode token, __COMPASS_SERVER_URL__ present only in client mode, and // JSON-escaping of a hostile server URL so it cannot break out of the script. -// (The embedded-mode arm of shellStartupJS is covered in embedded_test.go.) func TestShellStartupJS(t *testing.T) { t.Run("client injects mode and server url", func(t *testing.T) { js, err := shellStartupJS(appconfig.ModeClient.String(), "https://remote.example:8443") @@ -184,6 +183,23 @@ func TestShellStartupJS(t *testing.T) { } }) + t.Run("embedded injects mode and omits the server-url global", func(t *testing.T) { + // In embedded mode the app supervises a local stack and has no + // server_url, so shellStartupJS must emit only the mode token and MUST + // NOT emit __COMPASS_SERVER_URL__ (the branch at shellStartupJS's + // `if mode == client` gate). A non-empty serverURL argument is ignored. + js, err := shellStartupJS(appconfig.ModeEmbedded.String(), "https://ignored.example:8443") + if err != nil { + t.Fatalf("shellStartupJS err = %v, want nil", err) + } + if !strings.Contains(js, `window.__COMPASS_MODE__="embedded";`) { + t.Errorf("embedded JS = %q, want the embedded mode global", js) + } + if strings.Contains(js, "__COMPASS_SERVER_URL__") { + t.Errorf("embedded JS = %q, must not emit the server-url global", js) + } + }) + t.Run("hostile server url is JSON-escaped, not a breakout", func(t *testing.T) { hostile := `https://x/"+alert(1)+"