From a79ee95bcb2a8245f813fbc71c9ddb9374c05116 Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 6 Oct 2026 02:41:02 -0400 Subject: [PATCH 1/5] refactor(compass-app): extract embedded launch pipeline into internal/embedded Move the host-independent embedded pipeline (preflight, compass-stack up/down exec, WhoAmI over UDS, image and stack-binary resolution, the bring-up and teardown timeouts) from cmd/compass-app into a new go/internal/embedded package. Behaviour is unchanged; the app keeps runEmbedded, the quit controller, and the socket/mode resolvers. This gives the embedded launch a non-cgo package that an e2e test can drive without the Wails shell. Refs: RIG-4413 Co-authored-by: Matt Wilkinson --- app-bundle/SMOKE.md | 20 +- go/cmd/compass-app/embedded_launch_test.go | 198 ++++++++++++ go/cmd/compass-app/lifecycle.go | 17 +- go/cmd/compass-app/lifecycle_test.go | 18 +- go/cmd/compass-app/main.go | 85 +++-- go/cmd/compass-app/main_test.go | 12 +- go/cmd/compass-app/resolve_test.go | 66 ++++ .../embedded}/embedded.go | 194 ++++++------ .../embedded}/embedded_path_test.go | 4 +- .../embedded}/embedded_test.go | 295 ++---------------- .../embedded}/machine.go | 6 +- .../embedded}/machine_test.go | 4 +- .../embedded}/preflight_adapters.go | 6 +- 13 files changed, 476 insertions(+), 449 deletions(-) create mode 100644 go/cmd/compass-app/embedded_launch_test.go create mode 100644 go/cmd/compass-app/resolve_test.go rename go/{cmd/compass-app => internal/embedded}/embedded.go (73%) rename go/{cmd/compass-app => internal/embedded}/embedded_path_test.go (97%) rename go/{cmd/compass-app => internal/embedded}/embedded_test.go (67%) rename go/{cmd/compass-app => internal/embedded}/machine.go (99%) rename go/{cmd/compass-app => internal/embedded}/machine_test.go (99%) rename go/{cmd/compass-app => internal/embedded}/preflight_adapters.go (93%) diff --git a/app-bundle/SMOKE.md b/app-bundle/SMOKE.md index 9754e4538..64fd508e8 100644 --- a/app-bundle/SMOKE.md +++ b/app-bundle/SMOKE.md @@ -32,15 +32,15 @@ real agent container. The packaged-app smoke therefore remains manual. Embedded mode is the zero-config path. The app runs host preflight, brings up the local stack, resolves the caller identity, and then opens the board. The pipeline order is preflight, `compass-stack up`, then `WhoAmI` -(`runEmbedded` in `go/cmd/compass-app/embedded.go`: "pipeline ... WhoAmI"). +(`Pipeline.Run` in `go/internal/embedded/embedded.go`: "preflight → stack up → WhoAmI"). ### 1. Check embedded prerequisites Embedded mode requires Linux or macOS, rootless podman, and podman 4.3 or newer. These are fatal host checks. The agent image is checked locally but is pulled from GHCR by the stack when it is missing -(`Deps.Run` in `go/internal/preflight/preflight.go`, `runEmbedded` in -`go/cmd/compass-app/embedded.go`). Confirm rootless podman and the +(`Deps.Run` in `go/internal/preflight/preflight.go`, `Pipeline.Run` in +`go/internal/embedded/embedded.go`). Confirm rootless podman and the image before the smoke to avoid a cold pull: ```bash @@ -90,7 +90,7 @@ so residue from an unpinned launch is not mistaken for a clean teardown The stack binary resolution order is the `--compass-stack` flag, `COMPASS_STACK_BIN`, a `compass-stack` sibling of the running `compass-app`, -then `PATH` (`resolveStackBin` in `go/cmd/compass-app/embedded.go`). For this smoke, do not +then `PATH` (`ResolveStackBin` in `go/internal/embedded/embedded.go`). For this smoke, do not pass `--compass-stack` and require all launch overrides to be unset: ```bash @@ -105,9 +105,9 @@ from outside the bundle. `COMPASS_DATABASE_DSN` must also be clear so the smoke uses the bundle's state-directory database configuration. The app resolves `compass-stack` as a sibling of the running `compass-app` -executable, preferred over PATH (`resolveStackBin` in `go/cmd/compass-app/embedded.go`), and +executable, preferred over PATH (`ResolveStackBin` in `go/internal/embedded/embedded.go`), and prepends that same `bin/` directory for the supervised sidecars -(`prependExecDirToPath` in `go/cmd/compass-app/embedded.go`). So the bundle's staged +(`prependExecDirToPath` in `go/internal/embedded/embedded.go`). So the bundle's staged `compass-stack` wins even when an ambient one is on PATH, which is what the launch below relies on. @@ -144,16 +144,16 @@ With those flags the app invokes this stack command: compass-stack up --state-dir --image ghcr.io/rigelbuild/compass-agent:latest --socket ``` -`stackUpArgs` passes only `up`, `--state-dir`, `--image`, and `--socket` -(`stackUpArgs` in `go/cmd/compass-app/embedded.go`). It deliberately does not pass +`StackUpArgs` passes only `up`, `--state-dir`, `--image`, and `--socket` +(`StackUpArgs` in `go/internal/embedded/embedded.go`). It deliberately does not pass `--database`, `--postgres-image`, `--collector-image`, or `--listen`. The image ref is the locked GHCR default unless `--image` or `$COMPASS_AGENT_IMAGE` -overrides it (`resolveImage` in `go/cmd/compass-app/embedded.go`). +overrides it (`ResolveImage` in `go/internal/embedded/embedded.go`). ### 4. Confirm the embedded board and run one session Wait for the app to bring the stack to Ready. It then resolves the caller with -`WhoAmI` over the local socket (`runEmbedded` in `go/cmd/compass-app/embedded.go`). +`WhoAmI` over the local socket (`Pipeline.Run` in `go/internal/embedded/embedded.go`). Confirm that the app opens the board directly, without a client connect screen or bearer entry. Embedded mode has no client `server_url` or `ca_cert` configuration (`Parse` in `go/internal/appconfig/appconfig.go`: diff --git a/go/cmd/compass-app/embedded_launch_test.go b/go/cmd/compass-app/embedded_launch_test.go new file mode 100644 index 000000000..c53bb02bf --- /dev/null +++ b/go/cmd/compass-app/embedded_launch_test.go @@ -0,0 +1,198 @@ +//go:build (linux && gtk4) || darwin + +package main + +// App-side embedded launch gate: runEmbedded runs the pipeline and wires the +// quit controller, exercised with injected seams and no real exec. + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/RigelBuild/compass/go/internal/embedded" +) + +// baseParams is a representative resolved launch input the argv/dial assertions +// key off. The socket is a fixed path (the tests never dial it except in the +// WhoAmI-server case, which overrides it). +var baseParams = embedded.Params{ + Socket: "/run/compass/server.sock", + StateDir: "/state/compass", + Image: "ghcr.io/rigelbuild/compass-agent:latest", +} + +// stubPipeline builds an embedded.Pipeline whose three seams are deterministic +// stubs, recording what the orchestration invoked. Each seam defaults to a +// success no-op; a test overrides the ones it drives. +type recorder struct { + preflightCalled bool + stackUpCalled bool + stackUpArgs []string + whoAmICalled bool + whoAmISocket string +} + +func stubPipeline(rec *recorder, preflightErr, stackUpErr, whoAmIErr error, accountID string) embedded.Pipeline { + return embedded.Pipeline{ + Preflight: func(_ context.Context) error { + rec.preflightCalled = true + return preflightErr + }, + StackUp: func(_ context.Context, args []string) error { + rec.stackUpCalled = true + rec.stackUpArgs = args + return stackUpErr + }, + WhoAmI: func(_ context.Context, socket string) (string, error) { + rec.whoAmICalled = true + rec.whoAmISocket = socket + return accountID, whoAmIErr + }, + } +} + +// runEmbeddedStub runs runEmbedded with a recording stackDown seam so the quit +// controller wiring is observable without a real exec. +func runEmbeddedStub( + t *testing.T, pipeline embedded.Pipeline, +) (string, *quitController, error) { + t.Helper() + stackDown := func(_ context.Context, _ []string) error { return nil } + return runEmbedded(context.Background(), pipeline, baseParams, stackDown) +} + +// TestRunEmbeddedHappyPath: embedded mode runs preflight → stack up → WhoAmI in +// order, passes the SAME socket to the dial that the argv carries, returns the +// resolved account id, and builds a quit controller wired to the params. Asserting +// the argv (up, --socket, --state-dir, --image) is the stack-invocation contract; +// asserting whoAmISocket == socket is the single-socket invariant (the value +// passed to --socket IS the value dialed). +func TestRunEmbeddedHappyPath(t *testing.T) { + rec := &recorder{} + pipeline := stubPipeline(rec, nil, nil, nil, "acc-42") + + id, quitter, err := runEmbeddedStub(t, pipeline) + if err != nil { + t.Fatalf("embedded happy path err = %v, want nil", err) + } + if id != "acc-42" { + t.Errorf("account id = %q, want acc-42", id) + } + if quitter == nil { + t.Fatal("embedded mode returned a nil quit controller, want one wired to the stack teardown") + } + if quitter.params != baseParams { + t.Errorf("quit controller params = %+v, want %+v", quitter.params, baseParams) + } + if !rec.preflightCalled || !rec.stackUpCalled || !rec.whoAmICalled { + t.Fatalf("not every stage ran: %+v", rec) + } + assertArg(t, rec.stackUpArgs, "up") + assertArgPair(t, rec.stackUpArgs, "--socket", baseParams.Socket) + assertArgPair(t, rec.stackUpArgs, "--state-dir", baseParams.StateDir) + assertArgPair(t, rec.stackUpArgs, "--image", baseParams.Image) + if rec.whoAmISocket != baseParams.Socket { + t.Errorf("WhoAmI dialed %q, want the SAME socket passed to --socket %q", + rec.whoAmISocket, baseParams.Socket) + } +} + +// TestRunEmbeddedPreflightShortCircuits: a preflight failure returns the +// aggregated legible error VERBATIM, never proceeds to stack-up or WhoAmI, and +// builds no quit controller. Mutation that reddens it: running the checks after a +// failure, or reformatting Results.Err's copy. +func TestRunEmbeddedPreflightShortCircuits(t *testing.T) { + rec := &recorder{} + preflightErr := errors.New("embedded-mode preflight failed:\n - windows is not supported") + pipeline := stubPipeline(rec, preflightErr, nil, nil, "acc-x") + + id, quitter, err := runEmbeddedStub(t, pipeline) + if !errors.Is(err, preflightErr) { + t.Fatalf("preflight-fail err = %v, want the preflight error verbatim", err) + } + if id != "" { + t.Errorf("account id = %q, want empty on preflight failure", id) + } + if quitter != nil { + t.Error("preflight failure returned a quit controller, want nil") + } + if !rec.preflightCalled { + t.Error("preflight did not run") + } + if rec.stackUpCalled || rec.whoAmICalled { + t.Errorf("pipeline proceeded past a failed preflight: %+v", rec) + } +} + +// TestRunEmbeddedStackUpFails: a non-zero compass-stack up exit is surfaced and +// the pipeline stops before WhoAmI. The stackUp seam already folds stderr into +// its error (see TestRunStackUpNonZeroExitSurfacesStderr); here the contract is +// that runEmbedded propagates it and does not dial. +func TestRunEmbeddedStackUpFails(t *testing.T) { + rec := &recorder{} + stackErr := errors.New("compass-stack up failed: exit status 1: postgres refused") + pipeline := stubPipeline(rec, nil, stackErr, nil, "acc-x") + + id, quitter, err := runEmbeddedStub(t, pipeline) + if !errors.Is(err, stackErr) { + t.Fatalf("stack-up-fail err = %v, want the stack-up error", err) + } + if id != "" { + t.Errorf("account id = %q, want empty on stack-up failure", id) + } + if quitter != nil { + t.Error("stack-up failure returned a quit controller, want nil") + } + if rec.whoAmICalled { + t.Error("pipeline dialed WhoAmI after a failed stack-up") + } +} + +// TestRunEmbeddedWhoAmIFails: a WhoAmI error is surfaced (wrapped with the socket +// for context) and no account id is returned. Mutation that reddens it: +// swallowing the WhoAmI error and returning an empty id as success. +func TestRunEmbeddedWhoAmIFails(t *testing.T) { + rec := &recorder{} + whoErr := errors.New("connect: connection refused") + pipeline := stubPipeline(rec, nil, nil, whoErr, "") + + id, quitter, err := runEmbeddedStub(t, pipeline) + if !errors.Is(err, whoErr) { + t.Fatalf("whoami-fail err = %v, want the WhoAmI error wrapped", err) + } + if id != "" { + t.Errorf("account id = %q, want empty on WhoAmI failure", id) + } + if quitter != nil { + t.Error("WhoAmI failure returned a quit controller, want nil") + } + if !strings.Contains(err.Error(), baseParams.Socket) { + t.Errorf("WhoAmI error %q does not name the socket for context", err.Error()) + } +} + +// assertArg fails unless want appears as a token in args. +func assertArg(t *testing.T, args []string, want string) { + t.Helper() + if !slices.Contains(args, want) { + t.Errorf("argv %v missing token %q", args, want) + } +} + +// assertArgPair fails unless flag is immediately followed by value in args. +func assertArgPair(t *testing.T, args []string, flag, value string) { + t.Helper() + for i, a := range args { + if a == flag { + if i+1 < len(args) && args[i+1] == value { + return + } + t.Errorf("argv %v: flag %q not followed by %q", args, flag, value) + return + } + } + t.Errorf("argv %v missing flag %q", args, flag) +} diff --git a/go/cmd/compass-app/lifecycle.go b/go/cmd/compass-app/lifecycle.go index ebd76ec40..fe551399b 100644 --- a/go/cmd/compass-app/lifecycle.go +++ b/go/cmd/compass-app/lifecycle.go @@ -26,18 +26,9 @@ import ( "context" "log/slog" "time" -) - -// stackDownTimeout bounds the explicit teardown (compass-stack down: attach, -// SIGTERM the child tree, wait the server drain, release the lock). The bring-up -// context is already cancelled by the time the window is open, so -// stopStackAndQuit roots a FRESH bounded context off the caller's rather than -// reusing it. -const stackDownTimeout = 60 * time.Second -// stackDownCancelGrace is how long a timed-out down gets after SIGTERM to -// rewrite its survivor record before os/exec escalates to SIGKILL. -const stackDownCancelGrace = 10 * time.Second + "github.com/RigelBuild/compass/go/internal/embedded" +) // quitController is the explicit "Quit and stop stack" orchestration over its // injected effects. It holds the teardown seam (stackDown), the argv inputs @@ -47,7 +38,7 @@ const stackDownCancelGrace = 10 * time.Second // verified with no real exec and no display. type quitController struct { stackDown func(ctx context.Context, args []string) error - params embeddedParams + params embedded.Params quit func() timeout time.Duration logger *slog.Logger @@ -69,7 +60,7 @@ func (c quitController) stopStackAndQuit(ctx context.Context) { } downCtx, cancel := context.WithTimeout(ctx, c.timeout) defer cancel() - if err := c.stackDown(downCtx, stackDownArgs(c.params)); err != nil { + if err := c.stackDown(downCtx, embedded.StackDownArgs(c.params)); err != nil { // Quit-anyway (OQ-6): log and fall through to quit. logger.Error("stopping the embedded stack failed; quitting anyway "+ "(the stack lingers, which is the safe plain-quit default)", "error", err) diff --git a/go/cmd/compass-app/lifecycle_test.go b/go/cmd/compass-app/lifecycle_test.go index 6ca66671c..2df92bd6d 100644 --- a/go/cmd/compass-app/lifecycle_test.go +++ b/go/cmd/compass-app/lifecycle_test.go @@ -14,6 +14,8 @@ import ( "slices" "testing" "time" + + "github.com/RigelBuild/compass/go/internal/embedded" ) // TestStackDownArgs: the pure argv builder emits the exact `compass-stack down` @@ -21,16 +23,16 @@ import ( // (compass-stack recomputes the default DSN from --state-dir) and --linger (down // is not lingerable). func TestStackDownArgs(t *testing.T) { - args := stackDownArgs(baseParams) + args := embedded.StackDownArgs(baseParams) want := []string{ "down", - "--state-dir", baseParams.stateDir, - "--image", baseParams.image, - "--socket", baseParams.socket, + "--state-dir", baseParams.StateDir, + "--image", baseParams.Image, + "--socket", baseParams.Socket, } if !slices.Equal(args, want) { - t.Errorf("stackDownArgs = %v, want %v", args, want) + t.Errorf("StackDownArgs = %v, want %v", args, want) } if slices.Contains(args, "--database") { t.Errorf("argv carries --database, want it omitted (compass-stack defaults the DSN): %v", args) @@ -52,12 +54,12 @@ func TestStopStackAndQuitHappyPath(t *testing.T) { }, params: baseParams, quit: func() { quitCount++ }, - timeout: stackDownTimeout, + timeout: embedded.StackDownTimeout, } c.stopStackAndQuit(context.Background()) - if want := stackDownArgs(baseParams); !slices.Equal(gotArgs, want) { + if want := embedded.StackDownArgs(baseParams); !slices.Equal(gotArgs, want) { t.Errorf("stackDown argv = %v, want %v", gotArgs, want) } if quitCount != 1 { @@ -76,7 +78,7 @@ func TestStopStackAndQuitQuitsAnywayOnDownFailure(t *testing.T) { }, params: baseParams, quit: func() { quitCount++ }, - timeout: stackDownTimeout, + timeout: embedded.StackDownTimeout, } c.stopStackAndQuit(context.Background()) diff --git a/go/cmd/compass-app/main.go b/go/cmd/compass-app/main.go index 46cfc3d27..637c7cd16 100644 --- a/go/cmd/compass-app/main.go +++ b/go/cmd/compass-app/main.go @@ -30,47 +30,15 @@ import ( "log/slog" "os" "path/filepath" - "runtime" - "time" "github.com/RigelBuild/compass/go/internal/appconfig" "github.com/RigelBuild/compass/go/internal/bridge" + "github.com/RigelBuild/compass/go/internal/embedded" "github.com/RigelBuild/compass/go/internal/tokenstore" "github.com/wailsapp/wails/v3/pkg/application" "github.com/wailsapp/wails/v3/pkg/events" ) -// bringUpTimeout bounds the whole embedded bring-up (preflight + compass-stack -// up + WhoAmI) as a backstop against a wedged launch; app.Run() itself is not -// context-bound. It is generous because a cold first run pulls THREE images — -// the agent image from GHCR plus the stock postgres and collector images -// (DL-260) — before the stack reaches Ready, so the window covers three -// sequential registry pulls, not one. -// -// On darwin the window is wider still. The machine ensure step runs inside it, -// and a cold `podman machine init` downloads a VM image before any of the -// above starts — minutes on its own, on a link whose speed we do not control. -// A budget that cannot fit the work it wraps is not a backstop; it is a -// deadline the first launch on a fresh Mac loses every time, and the error it -// produces names the timeout rather than the download. So darwin gets a window -// sized for cold provisioning plus the same three pulls. Both remain backstops -// against a wedge, not performance targets. -// -// The configured embedded bring-up runs before a window opens. First-run setup -// only runs preflight inside the visible chooser and saves the choice; the next -// launch takes the configured bring-up path. -var bringUpTimeout = bringUpTimeoutFor(runtime.GOOS) - -// bringUpTimeoutFor returns the bring-up budget for the given host OS. It takes -// the OS as a parameter rather than reading runtime.GOOS so the per-OS choice -// is unit-testable from any host. -func bringUpTimeoutFor(goos string) time.Duration { - if goos == "darwin" { - return 15 * time.Minute - } - return 180 * time.Second -} - func main() { if err := run(); err != nil { slog.Error("compass-app exited with an error", "error", err) @@ -105,7 +73,7 @@ func run() error { "executable, then compass-stack on $PATH.") imageFlag := flag.String("image", "", "Agent container image ref for the embedded stack. Defaults to "+ - "$COMPASS_AGENT_IMAGE, then "+defaultAgentImage+".") + "$COMPASS_AGENT_IMAGE, then "+embedded.DefaultAgentImage+".") flag.Parse() socket := resolveSocket(*socketFlag) @@ -125,12 +93,12 @@ func run() error { dialog *dialogService ) if setupMode { - svc, setup, dialog, err = newSetupServices(stateDir, resolveImage(*imageFlag)) + svc, setup, dialog, err = newSetupServices(stateDir, embedded.ResolveImage(*imageFlag)) if err != nil { return err } } else { - svc, quitter, err = launch(cfg, socket, stateDir, resolveImage(*imageFlag), stackBinFlag) + svc, quitter, err = launch(cfg, socket, stateDir, embedded.ResolveImage(*imageFlag), stackBinFlag) if err != nil { return err } @@ -273,12 +241,12 @@ func newSetupServices(stateDir, image string) (*bridgeService, *setupService, *d picks := &caPicks{} wiring := &setupWiring{configPath: configPath, gate: gate, picks: picks} svc := newSetupBridgeService(nil, tokenstore.New(stateDir), wiring) - preflight := realPreflight(image) + preflight := embedded.RealPreflight(image) setup := &setupService{ gate: gate, svc: svc, preflight: func(ctx context.Context) error { - ctx, cancel := context.WithTimeout(ctx, bringUpTimeout) + ctx, cancel := context.WithTimeout(ctx, embedded.BringUpTimeout) defer cancel() return preflight(ctx) }, @@ -297,23 +265,23 @@ func launch( ) (*bridgeService, *quitController, error) { switch cfg.Mode { case appconfig.ModeEmbedded: - stackBin, err := resolveStackBin(*stackBinFlag) + stackBin, err := embedded.ResolveStackBin(*stackBinFlag) if err != nil { return nil, nil, err } - pipeline := embeddedPipeline{ - preflight: realPreflight(image), - stackUp: runStackUp(stackBin), - whoAmI: whoAmIOverUDS, + pipeline := embedded.Pipeline{ + Preflight: embedded.RealPreflight(image), + StackUp: embedded.RunStackUp(stackBin), + WhoAmI: embedded.WhoAmIOverUDS, } - params := embeddedParams{socket: socket, stateDir: stateDir, image: image} + params := embedded.Params{Socket: socket, StateDir: stateDir, Image: image} // The embedded bring-up (preflight → stack up → WhoAmI) runs BEFORE the // window opens, under a bounded context. launch() is invoked once from run() // with no context upstream, so this context.Background() is the sanctioned // root of the bring-up pipeline, not a mid-tree re-root. - bringUpCtx, cancel := context.WithTimeout(context.Background(), bringUpTimeout) - accountID, quitter, err := runEmbedded(bringUpCtx, pipeline, params, runStackDown(stackBin)) + bringUpCtx, cancel := context.WithTimeout(context.Background(), embedded.BringUpTimeout) + accountID, quitter, err := runEmbedded(bringUpCtx, pipeline, params, embedded.RunStackDown(stackBin)) cancel() if err != nil { return nil, nil, err @@ -419,3 +387,28 @@ func distDirForExecutable(exe string) string { } return filepath.Join(dir, "dist") } + +// runEmbedded runs the embedded-mode launch and builds the embedded-only quit +// controller. It runs the pipeline (preflight → stack up → WhoAmI) and, on +// success, returns the resolved caller account id together with a *quitController +// wired to the injected stackDown seam (its quit func is wired to app.Quit by +// run() once the app exists). resolveStackBin and this controller are embedded +// concerns only: a client-only install has no compass-stack binary and no stack +// to stop, so neither may gate a client launch (design §T5.6). +func runEmbedded( + ctx context.Context, + pipeline embedded.Pipeline, + params embedded.Params, + stackDown func(ctx context.Context, args []string) error, +) (string, *quitController, error) { + accountID, err := pipeline.Run(ctx, params) + if err != nil { + return "", nil, err + } + quitter := &quitController{ + stackDown: stackDown, + params: params, + timeout: embedded.StackDownTimeout, + } + return accountID, quitter, nil +} diff --git a/go/cmd/compass-app/main_test.go b/go/cmd/compass-app/main_test.go index 9f9db6e6e..c844bb61c 100644 --- a/go/cmd/compass-app/main_test.go +++ b/go/cmd/compass-app/main_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "testing" "time" + + "github.com/RigelBuild/compass/go/internal/embedded" ) // TestDistDirForExecutable pins the packaging-layout dist resolution: a macOS @@ -61,20 +63,20 @@ func TestDistDirForExecutable(t *testing.T) { // literal figures: re-tuning either budget is fine, collapsing the darwin one // back onto the linux one is the regression. func TestBringUpTimeoutBudgetsDarwinColdProvisioning(t *testing.T) { - linux := bringUpTimeoutFor("linux") - darwin := bringUpTimeoutFor("darwin") + linux := embedded.BringUpTimeoutFor("linux") + darwin := embedded.BringUpTimeoutFor("darwin") if darwin <= linux { - t.Errorf("bringUpTimeoutFor(darwin) = %v, not greater than linux %v; a cold "+ + t.Errorf("BringUpTimeoutFor(darwin) = %v, not greater than linux %v; a cold "+ "podman machine init cannot fit a linux-sized window", darwin, linux) } // A cold VM-image download plus three registry pulls does not fit in five // minutes on an ordinary connection. if darwin < 10*time.Minute { - t.Errorf("bringUpTimeoutFor(darwin) = %v, too tight for a cold machine init "+ + t.Errorf("BringUpTimeoutFor(darwin) = %v, too tight for a cold machine init "+ "plus three image pulls", darwin) } if linux <= 0 { - t.Errorf("bringUpTimeoutFor(linux) = %v, want a positive backstop", linux) + t.Errorf("BringUpTimeoutFor(linux) = %v, want a positive backstop", linux) } } diff --git a/go/cmd/compass-app/resolve_test.go b/go/cmd/compass-app/resolve_test.go new file mode 100644 index 000000000..400b62992 --- /dev/null +++ b/go/cmd/compass-app/resolve_test.go @@ -0,0 +1,66 @@ +//go:build (linux && gtk4) || darwin + +package main + +import ( + "path/filepath" + "testing" +) + +// TestResolveSocket: flag wins, then $COMPASS_SOCKET, then an ABSOLUTE +// $XDG_RUNTIME_DIR/compass/server.sock. A RELATIVE $XDG_RUNTIME_DIR is treated as +// unset and falls through to $HOME/.compass/server.sock — the determinism guard. +func TestResolveSocket(t *testing.T) { + t.Run("flag wins", func(t *testing.T) { + t.Setenv("COMPASS_SOCKET", "/env/server.sock") + if got := resolveSocket("/flag/server.sock"); got != "/flag/server.sock" { + t.Errorf("got %q, want the flag value", got) + } + }) + t.Run("env wins", func(t *testing.T) { + t.Setenv("COMPASS_SOCKET", "/env/server.sock") + t.Setenv("XDG_RUNTIME_DIR", "/xdg/run") + if got := resolveSocket(""); got != "/env/server.sock" { + t.Errorf("got %q, want the env value", got) + } + }) + t.Run("absolute XDG_RUNTIME_DIR", func(t *testing.T) { + t.Setenv("COMPASS_SOCKET", "") + xdg := t.TempDir() + t.Setenv("XDG_RUNTIME_DIR", xdg) + if got := resolveSocket(""); got != filepath.Join(xdg, "compass", "server.sock") { + t.Errorf("got %q, want %q", got, filepath.Join(xdg, "compass", "server.sock")) + } + }) + t.Run("relative XDG_RUNTIME_DIR falls through to HOME/.compass", func(t *testing.T) { + t.Setenv("COMPASS_SOCKET", "") + t.Setenv("XDG_RUNTIME_DIR", "rel/run") + home := t.TempDir() + t.Setenv("HOME", home) + if got := resolveSocket(""); got != filepath.Join(home, ".compass", "server.sock") { + t.Errorf("got %q, want %q (relative XDG_RUNTIME_DIR must fall through)", got, filepath.Join(home, ".compass", "server.sock")) + } + }) +} + +// TestResolveMode: flag wins, then $COMPASS_APP_MODE, then "" (no override). +func TestResolveMode(t *testing.T) { + t.Run("flag wins", func(t *testing.T) { + t.Setenv("COMPASS_APP_MODE", "client") + if got := resolveMode("embedded"); got != "embedded" { + t.Errorf("got %q, want the flag value", got) + } + }) + t.Run("env wins", func(t *testing.T) { + t.Setenv("COMPASS_APP_MODE", "client") + if got := resolveMode(""); got != "client" { + t.Errorf("got %q, want the env value", got) + } + }) + t.Run("both empty", func(t *testing.T) { + t.Setenv("COMPASS_APP_MODE", "") + if got := resolveMode(""); got != "" { + t.Errorf("got %q, want empty (no override)", got) + } + }) +} diff --git a/go/cmd/compass-app/embedded.go b/go/internal/embedded/embedded.go similarity index 73% rename from go/cmd/compass-app/embedded.go rename to go/internal/embedded/embedded.go index a710547c4..87263fe48 100644 --- a/go/cmd/compass-app/embedded.go +++ b/go/internal/embedded/embedded.go @@ -1,6 +1,4 @@ -//go:build (linux && gtk4) || darwin - -// The embedded-mode launch pipeline. Embedded mode wires the native shell +// Package embedded is the embedded-mode launch pipeline. Embedded mode wires the native shell // end-to-end before the window opens: host preflight → spawn and supervise the // private stack (via the compass-stack CLI) → learn the caller account id // (WhoAmI, DL-111) → hand the resolved socket + account id to the bridge/UI. It @@ -8,12 +6,12 @@ // probes, the compass-stack exec, the h2c-UDS WhoAmI dial) behind small injected // seams, so the orchestration is unit-testable without a real stack. // -// This file supervises the stack through the compass-stack BINARY, not by +// It supervises the stack through the compass-stack BINARY, not by // importing go/internal/stack: `compass-stack up` brings the stack to Ready and // exits 0 while the children keep running (fire-and-return), so the pipeline // runs it, waits for exit 0, and then dials the same socket it passed as // --socket. -package main +package embedded import ( "context" @@ -23,11 +21,12 @@ import ( "net" "net/http" "os" - "os/exec" //nolint:depguard // embedded stack supervisor: runs the compass-stack up/down binaries through the injected launch seams + "os/exec" //nolint:depguard // stack seam: runs the resolved compass-stack binary for up/down "path/filepath" "runtime" "strings" "syscall" + "time" "connectrpc.com/connect" @@ -36,15 +35,15 @@ import ( "github.com/RigelBuild/compass/go/internal/preflight" ) -// defaultAgentImage is the canonical agent image ref the embedded stack runs +// DefaultAgentImage is the canonical agent image ref the embedded stack runs // when no --image/$COMPASS_AGENT_IMAGE is supplied. The ref is locked // (docs/designs/infra/ci/compass-agent-image-publish/design.md, "the name/tag // contract"); the native app does not bundle the image (DL-112) — // compass-stack podman-pulls it from GHCR at first run. -const defaultAgentImage = "ghcr.io/rigelbuild/compass-agent:latest" +const DefaultAgentImage = "ghcr.io/rigelbuild/compass-agent:latest" // The compass-stack CLI flag names the embedded pipeline drives. Shared by -// stackUpArgs and stackDownArgs so the two argv builders cannot drift on a flag +// StackUpArgs and StackDownArgs so the two argv builders cannot drift on a flag // spelling (and so the strings are named once rather than repeated inline). const ( flagStateDir = "--state-dir" @@ -52,87 +51,62 @@ const ( flagSocket = "--socket" ) -// embeddedPipeline is the embedded-mode launch pipeline over its injected +// Pipeline is the embedded-mode launch pipeline over its injected // external effects. Each field is one genuine effect the real launch supplies // (preflight run, the compass-stack up exec, the WhoAmI dial); a test supplies // deterministic stubs, so the orchestration — order, short-circuit, and the // argv it builds — is verified with no real podman/stack/exec. -type embeddedPipeline struct { - // preflight runs the host precondition checks and folds any failures into a +type Pipeline struct { + // Preflight runs the host precondition checks and folds any failures into a // single legible error (the real seam wraps preflight.Deps.Run(...).Err()). - preflight func(ctx context.Context) error - // stackUp runs `compass-stack up` with the given argv and waits for it to + Preflight func(ctx context.Context) error + // StackUp runs `compass-stack up` with the given argv and waits for it to // exit 0 (fire-and-return); a non-zero exit is returned as an error carrying // the captured stderr. - stackUp func(ctx context.Context, args []string) error - // whoAmI dials the stack socket over h2c-UDS and returns the caller account + StackUp func(ctx context.Context, args []string) error + // WhoAmI dials the stack socket over h2c-UDS and returns the caller account // id (WhoAmI, DL-111 — server-derived, never supplied). - whoAmI func(ctx context.Context, socket string) (string, error) + WhoAmI func(ctx context.Context, socket string) (string, error) } -// embeddedParams is the resolved input to one embedded launch: the single socket +// Params is the resolved input to one embedded launch: the single socket // path the stack serves and the pipeline then dials, plus the stack argv inputs. // The argv omits --database so compass-stack recomputes the default DSN from // --state-dir (the app carries no second DSN copy — §A2 reconciliation 1). -type embeddedParams struct { - // socket is resolveSocket()'s result — the SAME value passed to +type Params struct { + // Socket is resolveSocket()'s result — the SAME value passed to // `--socket` and dialed for WhoAmI (and, upstream, the bridge pump). - socket string - // stateDir is the app state directory passed to `--state-dir`. - stateDir string - // image is the agent image ref passed to `--image`. - image string + Socket string + // StateDir is the app state directory passed to `--state-dir`. + StateDir string + // Image is the agent image ref passed to `--image`. + Image string } -// runEmbedded runs the embedded-mode launch and builds the embedded-only quit -// controller. It runs the pipeline (preflight → stack up → WhoAmI) and, on -// success, returns the resolved caller account id together with a *quitController -// wired to the injected stackDown seam (its quit func is wired to app.Quit by -// run() once the app exists). resolveStackBin and this controller are embedded -// concerns only: a client-only install has no compass-stack binary and no stack -// to stop, so neither may gate a client launch (design §T5.6). -func runEmbedded( - ctx context.Context, - pipeline embeddedPipeline, - params embeddedParams, - stackDown func(ctx context.Context, args []string) error, -) (string, *quitController, error) { - accountID, err := pipeline.run(ctx, params) - if err != nil { - return "", nil, err - } - quitter := &quitController{ - stackDown: stackDown, - params: params, - timeout: stackDownTimeout, - } - return accountID, quitter, nil -} - -// run executes the embedded launch in order: preflight → stack up → WhoAmI. A +// Run executes the embedded launch in order: preflight → stack up → WhoAmI. A // preflight failure short-circuits (the stack is never spawned) and returns the // aggregated legible error verbatim. On success it returns the resolved caller // account id. -func (p embeddedPipeline) run(ctx context.Context, params embeddedParams) (string, error) { - if err := p.preflight(ctx); err != nil { +func (p Pipeline) Run(ctx context.Context, params Params) (string, error) { + if err := p.Preflight(ctx); err != nil { return "", err } - args := stackUpArgs(params) - if err := p.stackUp(ctx, args); err != nil { + args := StackUpArgs(params) + if err := p.StackUp(ctx, args); err != nil { return "", err } - slog.Info("stack ready", "socket", params.socket) + slog.Info("stack ready", "socket", params.Socket) - accountID, err := p.whoAmI(ctx, params.socket) + accountID, err := p.WhoAmI(ctx, params.Socket) if err != nil { - return "", fmt.Errorf("resolving caller identity over %s: %w", params.socket, err) + return "", fmt.Errorf("resolving caller identity over %s: %w", params.Socket, err) } slog.Info("caller identity resolved", "account", accountID) return accountID, nil } -// stackUpArgs builds the `compass-stack up` argv from the resolved params. It is +// StackUpArgs builds the `compass-stack up` argv from the resolved params. It is // pure (no I/O, no exec) so the exact invocation is unit-testable without // running anything — mirroring cmd/compass-stack's pure resolveConfig. It passes // ONLY --state-dir/--image/--socket: --database is omitted (compass-stack @@ -140,12 +114,12 @@ func (p embeddedPipeline) run(ctx context.Context, params embeddedParams) (strin // duplicate that logic), and --postgres-image/--collector-image/--listen are // omitted so the CLI's defaults are the contract (the app re-learns nothing the // stack already owns — §A2 reconciliation 2). -func stackUpArgs(p embeddedParams) []string { +func StackUpArgs(p Params) []string { args := []string{ "up", - flagStateDir, p.stateDir, - flagImage, p.image, - flagSocket, p.socket, + flagStateDir, p.StateDir, + flagImage, p.Image, + flagSocket, p.Socket, } return args } @@ -184,15 +158,15 @@ func captureStderr(cmd *exec.Cmd) (read func() string, cleanup func(), err error return read, cleanup, nil } -// runStackUp is the real stackUp seam: it execs the compass-stack binary at bin +// RunStackUp is the real stackUp seam: it execs the compass-stack binary at bin // with the given argv and waits for it to exit 0 (up is fire-and-return, so // Run returning nil means the stack reached Ready and its children keep // running). A non-zero exit is surfaced with the captured stderr so the failure // copy is legible. -func runStackUp(bin string) func(ctx context.Context, args []string) error { +func RunStackUp(bin string) func(ctx context.Context, args []string) error { return func(ctx context.Context, args []string) error { - //nolint:gosec // G204: bin is operator/PATH-resolved (resolveStackBin) and - // the argv is pipeline-assembled (stackUpArgs), not user input. + //nolint:gosec // G204: bin is operator/PATH-resolved (ResolveStackBin) and + // the argv is pipeline-assembled (StackUpArgs), not user input. cmd := exec.CommandContext(ctx, bin, args...) cmd.Env = prependExecDirToPath(os.Environ(), filepath.Dir(bin)) stderr, cleanup, capErr := captureStderr(cmd) @@ -205,7 +179,7 @@ func runStackUp(bin string) func(ctx context.Context, args []string) error { return fmt.Errorf("compass-stack up exceeded the %s bring-up window "+ "(a cold first run pulls three images from their registries — the agent "+ "image from GHCR, the postgres and collector images from their stock "+ - "registries — which can take longer): %w", bringUpTimeout, err) + "registries — which can take longer): %w", BringUpTimeout, err) } if msg := stderr(); msg != "" { return fmt.Errorf("compass-stack up failed: %w: %s", err, msg) @@ -216,8 +190,8 @@ func runStackUp(bin string) func(ctx context.Context, args []string) error { } } -// stackDownArgs builds the `compass-stack down` argv from the resolved params. -// It mirrors stackUpArgs (pure, no I/O, no exec) so the exact teardown +// StackDownArgs builds the `compass-stack down` argv from the resolved params. +// It mirrors StackUpArgs (pure, no I/O, no exec) so the exact teardown // invocation is unit-testable without running anything. down parses the SAME // config flags as up, and its resolveConfig REQUIRES a non-empty --state-dir AND // --image (both rejected if empty), so --image is carried even though teardown @@ -226,25 +200,25 @@ func runStackUp(bin string) func(ctx context.Context, args []string) error { // recomputes the identical default DSN from --state-dir), and --linger is // omitted because down is not lingerable (down's whole job is to tear the stack // down, so a linger flag would be nonsense — compass-stack rejects it). -func stackDownArgs(p embeddedParams) []string { +func StackDownArgs(p Params) []string { args := []string{ "down", - flagStateDir, p.stateDir, - flagImage, p.image, - flagSocket, p.socket, + flagStateDir, p.StateDir, + flagImage, p.Image, + flagSocket, p.Socket, } return args } -// runStackDown is the real stackDown seam: it execs the compass-stack binary at +// RunStackDown is the real stackDown seam: it execs the compass-stack binary at // bin with the given argv and waits for it to exit 0 (down attaches to the live // stack, SIGTERMs the child tree, waits the server drain, and releases the // lock). A non-zero exit is surfaced with the captured stderr so the failure -// copy is legible — mirroring runStackUp's shape exactly. -func runStackDown(bin string) func(ctx context.Context, args []string) error { +// copy is legible — mirroring RunStackUp's shape exactly. +func RunStackDown(bin string) func(ctx context.Context, args []string) error { return func(ctx context.Context, args []string) error { - //nolint:gosec // G204: bin is operator/PATH-resolved (resolveStackBin) and - // the argv is pipeline-assembled (stackDownArgs), not user input. + //nolint:gosec // G204: bin is operator/PATH-resolved (ResolveStackBin) and + // the argv is pipeline-assembled (StackDownArgs), not user input. cmd := exec.CommandContext(ctx, bin, args...) // down has already consumed its teardown record, so a timeout must SIGTERM // it: SIGKILL would skip the survivor rewrite and leak the stack. @@ -259,7 +233,7 @@ func runStackDown(bin string) func(ctx context.Context, args []string) error { if err := cmd.Run(); err != nil { if ctx.Err() == context.DeadlineExceeded || errors.Is(err, context.DeadlineExceeded) { return fmt.Errorf("compass-stack down exceeded the %s teardown window "+ - "(attach, SIGTERM the child tree, wait the server drain): %w", stackDownTimeout, err) + "(attach, SIGTERM the child tree, wait the server drain): %w", StackDownTimeout, err) } if msg := stderr(); msg != "" { return fmt.Errorf("compass-stack down failed: %w: %s", err, msg) @@ -270,12 +244,12 @@ func runStackDown(bin string) func(ctx context.Context, args []string) error { } } -// whoAmIOverUDS is the real whoAmI seam: it dials the stack socket over +// WhoAmIOverUDS is the real whoAmI seam: it dials the stack socket over // prior-knowledge cleartext HTTP/2 (the same door compass-server serves) and // calls WhoAmI, returning the server-derived caller account id. The transport // shape mirrors internal/stack/adapters/health.go (the established h2c-UDS // connect dial). -func whoAmIOverUDS(ctx context.Context, socket string) (string, error) { +func WhoAmIOverUDS(ctx context.Context, socket string) (string, error) { protocols := new(http.Protocols) protocols.SetUnencryptedHTTP2(true) transport := &http.Transport{ @@ -299,7 +273,7 @@ func whoAmIOverUDS(ctx context.Context, socket string) (string, error) { return id, nil } -// resolveStackBin picks the compass-stack binary to supervise the stack with: +// ResolveStackBin picks the compass-stack binary to supervise the stack with: // the --compass-stack flag, else $COMPASS_STACK_BIN, else a compass-stack // sibling of the running executable (where a packaged build stages it, mirroring // resolveAssetsDir's beside-the-executable pattern), else compass-stack on @@ -307,7 +281,7 @@ func whoAmIOverUDS(ctx context.Context, socket string) (string, error) { // sidecar wins over any ambient compass-stack, while a dev-box build (no // sibling) still falls through to $PATH. A legible error names every place it // looked when none resolves. -func resolveStackBin(flagValue string) (string, error) { +func ResolveStackBin(flagValue string) (string, error) { if flagValue != "" { return flagValue, nil } @@ -360,20 +334,20 @@ func prependExecDirToPath(env []string, execDir string) []string { return append(out, "PATH="+execDir) } -// resolveImage picks the agent image ref: the --image flag, else +// ResolveImage picks the agent image ref: the --image flag, else // $COMPASS_AGENT_IMAGE (the same env compass-runner honors), else the locked // GHCR default. -func resolveImage(flagValue string) string { +func ResolveImage(flagValue string) string { if flagValue != "" { return flagValue } if env := os.Getenv("COMPASS_AGENT_IMAGE"); env != "" { return env } - return defaultAgentImage + return DefaultAgentImage } -// realPreflight builds the preflight seam over the real host-probe adapters, +// RealPreflight builds the preflight seam over the real host-probe adapters, // classified at this wiring boundary (see classifyPreflight). The DB probe and // the app-side DSN duplicate are gone (§A2 reconciliation 1): under DL-260 // postgres is a container the stack itself starts, so a pre-`up` reachability @@ -384,7 +358,7 @@ func resolveImage(flagValue string) string { // machine. The preflight core keys the check off GOOS and FAILS on darwin when // the adapter is nil, so this wiring cannot regress into a silently-skipped // check. -func realPreflight(image string) func(ctx context.Context) error { +func RealPreflight(image string) func(ctx context.Context) error { deps := realPreflightDeps(runtime.GOOS) params := preflight.Params{AgentImage: image} return func(ctx context.Context) error { @@ -442,3 +416,45 @@ func classifyPreflight(results preflight.Results) error { } return fatal.Err() } + +// StackDownTimeout bounds the explicit teardown (compass-stack down: attach, +// SIGTERM the child tree, wait the server drain, release the lock). The bring-up +// context is already cancelled by the time the window is open, so +// stopStackAndQuit roots a FRESH bounded context off the caller's rather than +// reusing it. +const StackDownTimeout = 60 * time.Second + +// stackDownCancelGrace is how long a timed-out down gets after SIGTERM to +// rewrite its survivor record before os/exec escalates to SIGKILL. +const stackDownCancelGrace = 10 * time.Second + +// BringUpTimeout bounds the whole embedded bring-up (preflight + compass-stack +// up + WhoAmI) as a backstop against a wedged launch; app.Run() itself is not +// context-bound. It is generous because a cold first run pulls THREE images — +// the agent image from GHCR plus the stock postgres and collector images +// (DL-260) — before the stack reaches Ready, so the window covers three +// sequential registry pulls, not one. +// +// On darwin the window is wider still. The machine ensure step runs inside it, +// and a cold `podman machine init` downloads a VM image before any of the +// above starts — minutes on its own, on a link whose speed we do not control. +// A budget that cannot fit the work it wraps is not a backstop; it is a +// deadline the first launch on a fresh Mac loses every time, and the error it +// produces names the timeout rather than the download. So darwin gets a window +// sized for cold provisioning plus the same three pulls. Both remain backstops +// against a wedge, not performance targets. +// +// The configured embedded bring-up runs before a window opens. First-run setup +// only runs preflight inside the visible chooser and saves the choice; the next +// launch takes the configured bring-up path. +var BringUpTimeout = BringUpTimeoutFor(runtime.GOOS) + +// BringUpTimeoutFor returns the bring-up budget for the given host OS. It takes +// the OS as a parameter rather than reading runtime.GOOS so the per-OS choice +// is unit-testable from any host. +func BringUpTimeoutFor(goos string) time.Duration { + if goos == "darwin" { + return 15 * time.Minute + } + return 180 * time.Second +} diff --git a/go/cmd/compass-app/embedded_path_test.go b/go/internal/embedded/embedded_path_test.go similarity index 97% rename from go/cmd/compass-app/embedded_path_test.go rename to go/internal/embedded/embedded_path_test.go index 003569d0a..22205b1fa 100644 --- a/go/cmd/compass-app/embedded_path_test.go +++ b/go/internal/embedded/embedded_path_test.go @@ -1,6 +1,4 @@ -//go:build (linux && gtk4) || darwin - -package main +package embedded // Sidecar PATH threading: prependExecDirToPath prepends the resolved // compass-stack's bundle dir to the child PATH so staged sidecars win diff --git a/go/cmd/compass-app/embedded_test.go b/go/internal/embedded/embedded_test.go similarity index 67% rename from go/cmd/compass-app/embedded_test.go rename to go/internal/embedded/embedded_test.go index b9472830d..84d5edf61 100644 --- a/go/cmd/compass-app/embedded_test.go +++ b/go/internal/embedded/embedded_test.go @@ -1,12 +1,10 @@ -//go:build (linux && gtk4) || darwin - -package main +package embedded // Embedded launch-pipeline gate. The pipeline is exercised through its Go // entrypoints with INJECTED effects — no real podman/compass-stack/exec — so // mode-select, preflight short-circuit, the exact compass-stack argv, the WhoAmI // hop, and the two error paths are all verified deterministically. The one seam -// wired to a real transport is whoAmIOverUDS, driven against a REAL in-process +// wired to a real transport is WhoAmIOverUDS, driven against a REAL in-process // compass.v1 WhoAmI server over h2c on a Unix socket (mirroring the // bridge-service gate's stubServer and internal/runner/e2e_transport_test.go's // UDS/connect pattern), so the h2c-UDS dial is proven on the wire it ships on. @@ -36,160 +34,10 @@ const embeddedTestTimeout = 5 * time.Second // baseParams is a representative resolved launch input the argv/dial assertions // key off. The socket is a fixed path (the tests never dial it except in the // WhoAmI-server case, which overrides it). -var baseParams = embeddedParams{ - socket: "/run/compass/server.sock", - stateDir: "/state/compass", - image: "ghcr.io/rigelbuild/compass-agent:latest", -} - -// stubPipeline builds an embeddedPipeline whose three seams are deterministic -// stubs, recording what the orchestration invoked. Each seam defaults to a -// success no-op; a test overrides the ones it drives. -type recorder struct { - preflightCalled bool - stackUpCalled bool - stackUpArgs []string - whoAmICalled bool - whoAmISocket string -} - -func stubPipeline(rec *recorder, preflightErr, stackUpErr, whoAmIErr error, accountID string) embeddedPipeline { - return embeddedPipeline{ - preflight: func(_ context.Context) error { - rec.preflightCalled = true - return preflightErr - }, - stackUp: func(_ context.Context, args []string) error { - rec.stackUpCalled = true - rec.stackUpArgs = args - return stackUpErr - }, - whoAmI: func(_ context.Context, socket string) (string, error) { - rec.whoAmICalled = true - rec.whoAmISocket = socket - return accountID, whoAmIErr - }, - } -} - -// runEmbeddedStub runs runEmbedded with a recording stackDown seam so the quit -// controller wiring is observable without a real exec. -func runEmbeddedStub( - t *testing.T, pipeline embeddedPipeline, -) (string, *quitController, error) { - t.Helper() - stackDown := func(_ context.Context, _ []string) error { return nil } - return runEmbedded(context.Background(), pipeline, baseParams, stackDown) -} - -// TestRunEmbeddedHappyPath: embedded mode runs preflight → stack up → WhoAmI in -// order, passes the SAME socket to the dial that the argv carries, returns the -// resolved account id, and builds a quit controller wired to the params. Asserting -// the argv (up, --socket, --state-dir, --image) is the stack-invocation contract; -// asserting whoAmISocket == socket is the single-socket invariant (the value -// passed to --socket IS the value dialed). -func TestRunEmbeddedHappyPath(t *testing.T) { - rec := &recorder{} - pipeline := stubPipeline(rec, nil, nil, nil, "acc-42") - - id, quitter, err := runEmbeddedStub(t, pipeline) - if err != nil { - t.Fatalf("embedded happy path err = %v, want nil", err) - } - if id != "acc-42" { - t.Errorf("account id = %q, want acc-42", id) - } - if quitter == nil { - t.Fatal("embedded mode returned a nil quit controller, want one wired to the stack teardown") - } - if quitter.params != baseParams { - t.Errorf("quit controller params = %+v, want %+v", quitter.params, baseParams) - } - if !rec.preflightCalled || !rec.stackUpCalled || !rec.whoAmICalled { - t.Fatalf("not every stage ran: %+v", rec) - } - assertArg(t, rec.stackUpArgs, "up") - assertArgPair(t, rec.stackUpArgs, "--socket", baseParams.socket) - assertArgPair(t, rec.stackUpArgs, "--state-dir", baseParams.stateDir) - assertArgPair(t, rec.stackUpArgs, "--image", baseParams.image) - if rec.whoAmISocket != baseParams.socket { - t.Errorf("WhoAmI dialed %q, want the SAME socket passed to --socket %q", - rec.whoAmISocket, baseParams.socket) - } -} - -// TestRunEmbeddedPreflightShortCircuits: a preflight failure returns the -// aggregated legible error VERBATIM, never proceeds to stack-up or WhoAmI, and -// builds no quit controller. Mutation that reddens it: running the checks after a -// failure, or reformatting Results.Err's copy. -func TestRunEmbeddedPreflightShortCircuits(t *testing.T) { - rec := &recorder{} - preflightErr := errors.New("embedded-mode preflight failed:\n - windows is not supported") - pipeline := stubPipeline(rec, preflightErr, nil, nil, "acc-x") - - id, quitter, err := runEmbeddedStub(t, pipeline) - if !errors.Is(err, preflightErr) { - t.Fatalf("preflight-fail err = %v, want the preflight error verbatim", err) - } - if id != "" { - t.Errorf("account id = %q, want empty on preflight failure", id) - } - if quitter != nil { - t.Error("preflight failure returned a quit controller, want nil") - } - if !rec.preflightCalled { - t.Error("preflight did not run") - } - if rec.stackUpCalled || rec.whoAmICalled { - t.Errorf("pipeline proceeded past a failed preflight: %+v", rec) - } -} - -// TestRunEmbeddedStackUpFails: a non-zero compass-stack up exit is surfaced and -// the pipeline stops before WhoAmI. The stackUp seam already folds stderr into -// its error (see TestRunStackUpNonZeroExitSurfacesStderr); here the contract is -// that runEmbedded propagates it and does not dial. -func TestRunEmbeddedStackUpFails(t *testing.T) { - rec := &recorder{} - stackErr := errors.New("compass-stack up failed: exit status 1: postgres refused") - pipeline := stubPipeline(rec, nil, stackErr, nil, "acc-x") - - id, quitter, err := runEmbeddedStub(t, pipeline) - if !errors.Is(err, stackErr) { - t.Fatalf("stack-up-fail err = %v, want the stack-up error", err) - } - if id != "" { - t.Errorf("account id = %q, want empty on stack-up failure", id) - } - if quitter != nil { - t.Error("stack-up failure returned a quit controller, want nil") - } - if rec.whoAmICalled { - t.Error("pipeline dialed WhoAmI after a failed stack-up") - } -} - -// TestRunEmbeddedWhoAmIFails: a WhoAmI error is surfaced (wrapped with the socket -// for context) and no account id is returned. Mutation that reddens it: -// swallowing the WhoAmI error and returning an empty id as success. -func TestRunEmbeddedWhoAmIFails(t *testing.T) { - rec := &recorder{} - whoErr := errors.New("connect: connection refused") - pipeline := stubPipeline(rec, nil, nil, whoErr, "") - - id, quitter, err := runEmbeddedStub(t, pipeline) - if !errors.Is(err, whoErr) { - t.Fatalf("whoami-fail err = %v, want the WhoAmI error wrapped", err) - } - if id != "" { - t.Errorf("account id = %q, want empty on WhoAmI failure", id) - } - if quitter != nil { - t.Error("WhoAmI failure returned a quit controller, want nil") - } - if !strings.Contains(err.Error(), baseParams.socket) { - t.Errorf("WhoAmI error %q does not name the socket for context", err.Error()) - } +var baseParams = Params{ + Socket: "/run/compass/server.sock", + StateDir: "/state/compass", + Image: "ghcr.io/rigelbuild/compass-agent:latest", } // TestStackUpArgsOmitsCLIDefaultedFlags: the pure argv builder passes ONLY @@ -199,7 +47,7 @@ func TestRunEmbeddedWhoAmIFails(t *testing.T) { // the contract and the app re-learns nothing the stack already owns (§A2 // reconciliation 2). func TestStackUpArgsOmitsCLIDefaultedFlags(t *testing.T) { - args := stackUpArgs(baseParams) + args := StackUpArgs(baseParams) for _, flag := range []string{ "--database", "--postgres-image", "--collector-image", "--listen", } { @@ -257,9 +105,9 @@ func TestWhoAmIOverUDSReturnsAccountID(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - id, err := whoAmIOverUDS(ctx, socket) + id, err := WhoAmIOverUDS(ctx, socket) if err != nil { - t.Fatalf("whoAmIOverUDS err = %v, want nil", err) + t.Fatalf("WhoAmIOverUDS err = %v, want nil", err) } if id != "acc-served" { t.Errorf("account id = %q, want acc-served", id) @@ -274,9 +122,9 @@ func TestWhoAmIOverUDSSurfacesError(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - id, err := whoAmIOverUDS(ctx, socket) + id, err := WhoAmIOverUDS(ctx, socket) if err == nil { - t.Fatal("whoAmIOverUDS err = nil, want the server's error surfaced") + t.Fatal("WhoAmIOverUDS err = nil, want the server's error surfaced") } if id != "" { t.Errorf("account id = %q, want empty on a WhoAmI error", id) @@ -292,7 +140,7 @@ func TestRunStackUpNonZeroExitSurfacesStderr(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - stackUp := runStackUp("/bin/sh") + stackUp := RunStackUp("/bin/sh") err := stackUp(ctx, []string{"-c", "echo 'boom on stderr' >&2; exit 1"}) if err == nil { t.Fatal("stackUp err = nil, want a non-zero-exit error") @@ -308,7 +156,7 @@ func TestRunStackUpZeroExitSucceeds(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - stackUp := runStackUp("/bin/sh") + stackUp := RunStackUp("/bin/sh") if err := stackUp(ctx, []string{"-c", "exit 0"}); err != nil { t.Fatalf("stackUp on a zero exit err = %v, want nil", err) } @@ -336,10 +184,10 @@ func TestRunStackUpReturnsWhileChildrenLinger(t *testing.T) { // A short-lived grandchild that outlives its parent and inherits stderr: the // exact fire-and-return shape of `compass-stack up`. sleep 5 is far longer - // than any correct runStackUp (which returns at the parent's exit, ~ms) and + // than any correct RunStackUp (which returns at the parent's exit, ~ms) and // well past the 1s assertion below, yet short enough that a regressed run's // leaked grandchild self-reaps in seconds rather than a minute. - stackUp := runStackUp("/bin/sh") + stackUp := RunStackUp("/bin/sh") start := time.Now() err := stackUp(ctx, []string{"-c", "sleep 5 & exit 0"}) elapsed := time.Since(start) @@ -356,14 +204,14 @@ func TestRunStackUpReturnsWhileChildrenLinger(t *testing.T) { } // TestRunStackDownNonZeroExitSurfacesStderr: the real stackDown seam surfaces a -// non-zero exit as an error carrying the child's stderr, mirroring runStackUp. +// non-zero exit as an error carrying the child's stderr, mirroring RunStackUp. // Driven with /bin/sh printing to stderr and exiting 1 — no real compass-stack // (the argv is not compass-stack's; only the exec+stderr contract is tested). func TestRunStackDownNonZeroExitSurfacesStderr(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - stackDown := runStackDown("/bin/sh") + stackDown := RunStackDown("/bin/sh") err := stackDown(ctx, []string{"-c", "echo 'down boom on stderr' >&2; exit 1"}) if err == nil { t.Fatal("stackDown err = nil, want a non-zero-exit error") @@ -379,7 +227,7 @@ func TestRunStackDownZeroExitSucceeds(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - stackDown := runStackDown("/bin/sh") + stackDown := RunStackDown("/bin/sh") if err := stackDown(ctx, []string{"-c", "exit 0"}); err != nil { t.Fatalf("stackDown on a zero exit err = %v, want nil", err) } @@ -407,7 +255,7 @@ func TestRunStackDownCancelSendsSIGTERM(t *testing.T) { }() script := "trap 'echo ok > \"$1\"; exit 3' TERM; echo > \"$2\"; while :; do sleep 1 & wait; done" - err := runStackDown("/bin/sh")(ctx, []string{"-c", script, "sh", marker, ready}) + err := RunStackDown("/bin/sh")(ctx, []string{"-c", script, "sh", marker, ready}) if err == nil { t.Fatal("stackDown err = nil, want the cancelled child's exit error") } @@ -597,7 +445,7 @@ func TestRunStackUpDeadlineExceededNamesBringUpWindow(t *testing.T) { ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) defer cancel() - stackUp := runStackUp("/bin/sh") + stackUp := RunStackUp("/bin/sh") err := stackUp(ctx, []string{"-c", "exit 0"}) if err == nil { t.Fatal("stackUp err = nil, want a deadline-exceeded error") @@ -615,9 +463,9 @@ func TestWhoAmIOverUDSRejectsEmptyAccountID(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() - id, err := whoAmIOverUDS(ctx, socket) + id, err := WhoAmIOverUDS(ctx, socket) if err == nil { - t.Fatal("whoAmIOverUDS err = nil, want an error on an empty account id") + t.Fatal("WhoAmIOverUDS err = nil, want an error on an empty account id") } if id != "" { t.Errorf("account id = %q, want empty when WhoAmI returns an empty id", id) @@ -631,7 +479,7 @@ func TestWhoAmIOverUDSRejectsEmptyAccountID(t *testing.T) { func TestResolveStackBin(t *testing.T) { t.Run("flag wins", func(t *testing.T) { t.Setenv("COMPASS_STACK_BIN", "/env/compass-stack") - got, err := resolveStackBin("/flag/compass-stack") + got, err := ResolveStackBin("/flag/compass-stack") if err != nil { t.Fatalf("err = %v, want nil", err) } @@ -641,7 +489,7 @@ func TestResolveStackBin(t *testing.T) { }) t.Run("env wins over PATH", func(t *testing.T) { t.Setenv("COMPASS_STACK_BIN", "/env/compass-stack") - got, err := resolveStackBin("") + got, err := ResolveStackBin("") if err != nil { t.Fatalf("err = %v, want nil", err) } @@ -652,7 +500,7 @@ func TestResolveStackBin(t *testing.T) { t.Run("not found names all four locations", func(t *testing.T) { t.Setenv("COMPASS_STACK_BIN", "") t.Setenv("PATH", "") - _, err := resolveStackBin("") + _, err := ResolveStackBin("") if err == nil { t.Fatal("err = nil, want a not-found error") } @@ -664,61 +512,25 @@ func TestResolveStackBin(t *testing.T) { }) } -// TestResolveSocket: flag wins, then $COMPASS_SOCKET, then an ABSOLUTE -// $XDG_RUNTIME_DIR/compass/server.sock. A RELATIVE $XDG_RUNTIME_DIR is treated as -// unset and falls through to $HOME/.compass/server.sock — the determinism guard. -func TestResolveSocket(t *testing.T) { - t.Run("flag wins", func(t *testing.T) { - t.Setenv("COMPASS_SOCKET", "/env/server.sock") - if got := resolveSocket("/flag/server.sock"); got != "/flag/server.sock" { - t.Errorf("got %q, want the flag value", got) - } - }) - t.Run("env wins", func(t *testing.T) { - t.Setenv("COMPASS_SOCKET", "/env/server.sock") - t.Setenv("XDG_RUNTIME_DIR", "/xdg/run") - if got := resolveSocket(""); got != "/env/server.sock" { - t.Errorf("got %q, want the env value", got) - } - }) - t.Run("absolute XDG_RUNTIME_DIR", func(t *testing.T) { - t.Setenv("COMPASS_SOCKET", "") - xdg := t.TempDir() - t.Setenv("XDG_RUNTIME_DIR", xdg) - if got := resolveSocket(""); got != filepath.Join(xdg, "compass", "server.sock") { - t.Errorf("got %q, want %q", got, filepath.Join(xdg, "compass", "server.sock")) - } - }) - t.Run("relative XDG_RUNTIME_DIR falls through to HOME/.compass", func(t *testing.T) { - t.Setenv("COMPASS_SOCKET", "") - t.Setenv("XDG_RUNTIME_DIR", "rel/run") - home := t.TempDir() - t.Setenv("HOME", home) - if got := resolveSocket(""); got != filepath.Join(home, ".compass", "server.sock") { - t.Errorf("got %q, want %q (relative XDG_RUNTIME_DIR must fall through)", got, filepath.Join(home, ".compass", "server.sock")) - } - }) -} - // TestResolveImage: flag wins, then $COMPASS_AGENT_IMAGE, then the locked GHCR // default. func TestResolveImage(t *testing.T) { t.Run("flag wins", func(t *testing.T) { t.Setenv("COMPASS_AGENT_IMAGE", "env/image:tag") - if got := resolveImage("flag/image:tag"); got != "flag/image:tag" { + if got := ResolveImage("flag/image:tag"); got != "flag/image:tag" { t.Errorf("got %q, want the flag value", got) } }) t.Run("env wins", func(t *testing.T) { t.Setenv("COMPASS_AGENT_IMAGE", "env/image:tag") - if got := resolveImage(""); got != "env/image:tag" { + if got := ResolveImage(""); got != "env/image:tag" { t.Errorf("got %q, want the env value", got) } }) t.Run("default", func(t *testing.T) { t.Setenv("COMPASS_AGENT_IMAGE", "") - if got := resolveImage(""); got != defaultAgentImage { - t.Errorf("got %q, want defaultAgentImage %q", got, defaultAgentImage) + if got := ResolveImage(""); got != DefaultAgentImage { + t.Errorf("got %q, want DefaultAgentImage %q", got, DefaultAgentImage) } }) // Pin the default to the canonical live GHCR owner. This guards the value @@ -726,53 +538,8 @@ func TestResolveImage(t *testing.T) { // ghcr.io/rigelbuild/compass-agent owner 403s post org-rename, so a // shipped app that fell back to it could not pull its agent image (RIG-1967). t.Run("default is the canonical rigelbuild ref", func(t *testing.T) { - if defaultAgentImage != "ghcr.io/rigelbuild/compass-agent:latest" { - t.Errorf("defaultAgentImage = %q, want the canonical rigelbuild ref", defaultAgentImage) + if DefaultAgentImage != "ghcr.io/rigelbuild/compass-agent:latest" { + t.Errorf("DefaultAgentImage = %q, want the canonical rigelbuild ref", DefaultAgentImage) } }) } - -// TestResolveMode: flag wins, then $COMPASS_APP_MODE, then "" (no override). -func TestResolveMode(t *testing.T) { - t.Run("flag wins", func(t *testing.T) { - t.Setenv("COMPASS_APP_MODE", "client") - if got := resolveMode("embedded"); got != "embedded" { - t.Errorf("got %q, want the flag value", got) - } - }) - t.Run("env wins", func(t *testing.T) { - t.Setenv("COMPASS_APP_MODE", "client") - if got := resolveMode(""); got != "client" { - t.Errorf("got %q, want the env value", got) - } - }) - t.Run("both empty", func(t *testing.T) { - t.Setenv("COMPASS_APP_MODE", "") - if got := resolveMode(""); got != "" { - t.Errorf("got %q, want empty (no override)", got) - } - }) -} - -// assertArg fails unless want appears as a token in args. -func assertArg(t *testing.T, args []string, want string) { - t.Helper() - if !slices.Contains(args, want) { - t.Errorf("argv %v missing token %q", args, want) - } -} - -// assertArgPair fails unless flag is immediately followed by value in args. -func assertArgPair(t *testing.T, args []string, flag, value string) { - t.Helper() - for i, a := range args { - if a == flag { - if i+1 < len(args) && args[i+1] == value { - return - } - t.Errorf("argv %v: flag %q not followed by %q", args, flag, value) - return - } - } - t.Errorf("argv %v missing flag %q", args, flag) -} diff --git a/go/cmd/compass-app/machine.go b/go/internal/embedded/machine.go similarity index 99% rename from go/cmd/compass-app/machine.go rename to go/internal/embedded/machine.go index 26c26b754..63f05e907 100644 --- a/go/cmd/compass-app/machine.go +++ b/go/internal/embedded/machine.go @@ -1,5 +1,3 @@ -//go:build (linux && gtk4) || darwin - // The podman-machine probe and ensure step behind an injected seam. On macOS the // podman CLI drives a Linux VM ("the machine") and a fresh Mac has no machine at // all, so embedded mode must both DETECT the machine's state and PROVISION it — @@ -20,7 +18,7 @@ // string are accepted; a missing field degrades, never panics), and an // unparseable answer is classified UNKNOWN, which is never ready. The failure // copy always names the podman command the operator can run themselves. -package main +package embedded import ( "context" @@ -288,7 +286,7 @@ func machineSocketReachable(ctx context.Context, d machineDeps, info machineInfo // // The init download is minutes long and runs under the caller's context, which // the embedded pipeline bounds with its bring-up window. On darwin that window -// is sized for a cold provision (bringUpTimeoutFor in main.go), so a healthy +// is sized for a cold provision (BringUpTimeoutFor), so a healthy // first run fits inside it. The copy on the failure path still names the init // command, so an operator who does exhaust the window gets something to run by // hand rather than a bare deadline error. diff --git a/go/cmd/compass-app/machine_test.go b/go/internal/embedded/machine_test.go similarity index 99% rename from go/cmd/compass-app/machine_test.go rename to go/internal/embedded/machine_test.go index 5e56814a7..5896bdf03 100644 --- a/go/cmd/compass-app/machine_test.go +++ b/go/internal/embedded/machine_test.go @@ -1,6 +1,4 @@ -//go:build (linux && gtk4) || darwin - -package main +package embedded import ( "context" diff --git a/go/cmd/compass-app/preflight_adapters.go b/go/internal/embedded/preflight_adapters.go similarity index 93% rename from go/cmd/compass-app/preflight_adapters.go rename to go/internal/embedded/preflight_adapters.go index 0b1dbe2d4..04f131a6c 100644 --- a/go/cmd/compass-app/preflight_adapters.go +++ b/go/internal/embedded/preflight_adapters.go @@ -1,5 +1,3 @@ -//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 @@ -7,12 +5,12 @@ // 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 +package embedded import ( "context" "fmt" - "os/exec" //nolint:depguard // embedded preflight adapters: podman info and podman image exists , the ref passed as one argv operand + "os/exec" //nolint:depguard // preflight seam: fixed-arg podman probes and LookPath "strings" "github.com/RigelBuild/compass/go/internal/runtime" From b412c77e459efd8701b3182eba523a6f46dbfb18 Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 6 Oct 2026 14:07:30 -0400 Subject: [PATCH 2/5] refactor(embedded): unexport stackUpArgs and fix moved-symbol comments stackUpArgs has no caller outside the package. Comments that named the pre-move identifiers now name the current ones. Refs: RIG-4413 Co-authored-by: Matt Wilkinson --- app-bundle/SMOKE.md | 4 ++-- go/cmd/compass-app/lifecycle_test.go | 2 +- go/internal/embedded/embedded.go | 12 ++++++------ go/internal/embedded/embedded_test.go | 8 ++++---- go/internal/embedded/preflight_adapters.go | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app-bundle/SMOKE.md b/app-bundle/SMOKE.md index 64fd508e8..41af6d446 100644 --- a/app-bundle/SMOKE.md +++ b/app-bundle/SMOKE.md @@ -144,8 +144,8 @@ With those flags the app invokes this stack command: compass-stack up --state-dir --image ghcr.io/rigelbuild/compass-agent:latest --socket ``` -`StackUpArgs` passes only `up`, `--state-dir`, `--image`, and `--socket` -(`StackUpArgs` in `go/internal/embedded/embedded.go`). It deliberately does not pass +`stackUpArgs` passes only `up`, `--state-dir`, `--image`, and `--socket` +(`stackUpArgs` in `go/internal/embedded/embedded.go`). It deliberately does not pass `--database`, `--postgres-image`, `--collector-image`, or `--listen`. The image ref is the locked GHCR default unless `--image` or `$COMPASS_AGENT_IMAGE` overrides it (`ResolveImage` in `go/internal/embedded/embedded.go`). diff --git a/go/cmd/compass-app/lifecycle_test.go b/go/cmd/compass-app/lifecycle_test.go index 2df92bd6d..11ff5323e 100644 --- a/go/cmd/compass-app/lifecycle_test.go +++ b/go/cmd/compass-app/lifecycle_test.go @@ -43,7 +43,7 @@ func TestStackDownArgs(t *testing.T) { } // TestStopStackAndQuitHappyPath: a successful teardown runs down with EXACTLY -// the stackDownArgs(params) argv and then quits the app exactly once. +// the StackDownArgs(params) argv and then quits the app exactly once. func TestStopStackAndQuitHappyPath(t *testing.T) { var gotArgs []string quitCount := 0 diff --git a/go/internal/embedded/embedded.go b/go/internal/embedded/embedded.go index 87263fe48..80ecf53af 100644 --- a/go/internal/embedded/embedded.go +++ b/go/internal/embedded/embedded.go @@ -43,7 +43,7 @@ import ( const DefaultAgentImage = "ghcr.io/rigelbuild/compass-agent:latest" // The compass-stack CLI flag names the embedded pipeline drives. Shared by -// StackUpArgs and StackDownArgs so the two argv builders cannot drift on a flag +// stackUpArgs and StackDownArgs so the two argv builders cannot drift on a flag // spelling (and so the strings are named once rather than repeated inline). const ( flagStateDir = "--state-dir" @@ -92,7 +92,7 @@ func (p Pipeline) Run(ctx context.Context, params Params) (string, error) { return "", err } - args := StackUpArgs(params) + args := stackUpArgs(params) if err := p.StackUp(ctx, args); err != nil { return "", err } @@ -106,7 +106,7 @@ func (p Pipeline) Run(ctx context.Context, params Params) (string, error) { return accountID, nil } -// StackUpArgs builds the `compass-stack up` argv from the resolved params. It is +// stackUpArgs builds the `compass-stack up` argv from the resolved params. It is // pure (no I/O, no exec) so the exact invocation is unit-testable without // running anything — mirroring cmd/compass-stack's pure resolveConfig. It passes // ONLY --state-dir/--image/--socket: --database is omitted (compass-stack @@ -114,7 +114,7 @@ func (p Pipeline) Run(ctx context.Context, params Params) (string, error) { // duplicate that logic), and --postgres-image/--collector-image/--listen are // omitted so the CLI's defaults are the contract (the app re-learns nothing the // stack already owns — §A2 reconciliation 2). -func StackUpArgs(p Params) []string { +func stackUpArgs(p Params) []string { args := []string{ "up", flagStateDir, p.StateDir, @@ -166,7 +166,7 @@ func captureStderr(cmd *exec.Cmd) (read func() string, cleanup func(), err error func RunStackUp(bin string) func(ctx context.Context, args []string) error { return func(ctx context.Context, args []string) error { //nolint:gosec // G204: bin is operator/PATH-resolved (ResolveStackBin) and - // the argv is pipeline-assembled (StackUpArgs), not user input. + // the argv is pipeline-assembled (stackUpArgs), not user input. cmd := exec.CommandContext(ctx, bin, args...) cmd.Env = prependExecDirToPath(os.Environ(), filepath.Dir(bin)) stderr, cleanup, capErr := captureStderr(cmd) @@ -191,7 +191,7 @@ func RunStackUp(bin string) func(ctx context.Context, args []string) error { } // StackDownArgs builds the `compass-stack down` argv from the resolved params. -// It mirrors StackUpArgs (pure, no I/O, no exec) so the exact teardown +// It mirrors stackUpArgs (pure, no I/O, no exec) so the exact teardown // invocation is unit-testable without running anything. down parses the SAME // config flags as up, and its resolveConfig REQUIRES a non-empty --state-dir AND // --image (both rejected if empty), so --image is carried even though teardown diff --git a/go/internal/embedded/embedded_test.go b/go/internal/embedded/embedded_test.go index 84d5edf61..32db70b46 100644 --- a/go/internal/embedded/embedded_test.go +++ b/go/internal/embedded/embedded_test.go @@ -47,7 +47,7 @@ var baseParams = Params{ // the contract and the app re-learns nothing the stack already owns (§A2 // reconciliation 2). func TestStackUpArgsOmitsCLIDefaultedFlags(t *testing.T) { - args := StackUpArgs(baseParams) + args := stackUpArgs(baseParams) for _, flag := range []string{ "--database", "--postgres-image", "--collector-image", "--listen", } { @@ -76,7 +76,7 @@ func (s *stubWhoAmIServer) WhoAmI( } // serveWhoAmI stands up a real h2c compass.v1 server on a UDS listener, torn -// down via t.Cleanup, and returns the socket path whoAmIOverUDS dials. +// down via t.Cleanup, and returns the socket path WhoAmIOverUDS dials. func serveWhoAmI(t *testing.T, srv *stubWhoAmIServer) string { t.Helper() socket := filepath.Join(t.TempDir(), "server.sock") @@ -165,7 +165,7 @@ func TestRunStackUpZeroExitSucceeds(t *testing.T) { // TestRunStackUpReturnsWhileChildrenLinger is the regression guard for the // fire-and-return hang: `compass-stack up` exits 0 once the stack is Ready while // its postgres/server/runner children keep running, and those children inherit -// the exec'd command's stderr. If runStackUp captured stderr into a bytes.Buffer +// the exec'd command's stderr. If RunStackUp captured stderr into a bytes.Buffer // (os/exec's pipe + copy-goroutine path), cmd.Wait would block until the pipe // hit EOF — which the lingering children hold open — so Run would hang for the // children's whole lifetime. Capturing to a temp *os.File (captureStderr) makes @@ -281,7 +281,7 @@ var classifyParams = preflight.Params{ } // classify runs deps and folds through the boundary classifier, the exact path -// realPreflight uses. +// RealPreflight uses. func classify(t *testing.T, deps preflight.Deps) error { t.Helper() return classifyPreflight(deps.Run(context.Background(), classifyParams)) diff --git a/go/internal/embedded/preflight_adapters.go b/go/internal/embedded/preflight_adapters.go index 04f131a6c..3dfda36b8 100644 --- a/go/internal/embedded/preflight_adapters.go +++ b/go/internal/embedded/preflight_adapters.go @@ -3,7 +3,7 @@ // 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) +// core seams; the pipeline's composition root (RealPreflight in embedded.go) // supplies them. package embedded From 07c14affc54598a20b9af155f6ee59bced512cce Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 6 Oct 2026 14:47:37 -0400 Subject: [PATCH 3/5] ci(darwin): run the moved machine and preflight tests from internal/embedded The darwin gate requires named PASS lines for the machine and preflight tests, which now live in go/internal/embedded. Refs: RIG-4413 Co-authored-by: Matt Wilkinson --- .github/workflows/ci.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8f072e39..2a9f46345 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1862,12 +1862,13 @@ jobs: # `podman machine init` can fit inside. # `-run` alone exits 0 when it matches nothing (a rename → false # green), so require each group's own PASS line — a rename or skip - # reds. The filter is explicit rather than the whole package because - # the package also holds GUI E2E tests that need a display. + # reds. The filter is explicit rather than the whole compass-app + # package because that package also holds GUI E2E tests that need a + # display; the machine and preflight tests live in internal/embedded. CGO_ENABLED=1 go -C go test -trimpath \ -run 'TestDistDirForExecutable|TestMachineReady|TestEnsureMachineReady|TestMachineResourceFloorIsExplicit|TestRealPreflightDeps|TestClassifyPreflight|TestBringUpTimeout' \ -count=1 -v \ - ./cmd/compass-app/ | tee /tmp/darwin-unit.log + ./cmd/compass-app/ ./internal/embedded/ | tee /tmp/darwin-unit.log for t in TestDistDirForExecutable \ TestMachineReadyRunning \ TestMachineReadyNoMachine \ From 5c1c023fe73a61a1978c7aba36d031ec9bea344b Mon Sep 17 00:00:00 2001 From: mintaka Date: Thu, 8 Oct 2026 19:39:49 -0400 Subject: [PATCH 4/5] test(embedded): read the SIGTERM gate FIFO to EOF before cancelling Closing the read end before the child echoes into the FIFO SIGPIPEd the child, so the trap never ran and the test failed about 1 in 10 runs under -race. Co-authored-by: Matt Wilkinson --- go/internal/embedded/embedded_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/go/internal/embedded/embedded_test.go b/go/internal/embedded/embedded_test.go index 32db70b46..3749be104 100644 --- a/go/internal/embedded/embedded_test.go +++ b/go/internal/embedded/embedded_test.go @@ -12,6 +12,7 @@ package embedded import ( "context" "errors" + "io" "net" "net/http" "os" @@ -246,9 +247,11 @@ func TestRunStackDownCancelSendsSIGTERM(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout) defer cancel() go func() { - // Opening the FIFO blocks until the child has armed its trap. + // Read to EOF: the child has armed its trap once it opens the FIFO, and + // closing before its write lands would SIGPIPE it instead of the cancel. f, err := os.Open(ready) if err == nil { + _, _ = io.Copy(io.Discard, f) _ = f.Close() // read end of a gate FIFO; nothing to flush } cancel() From 50016dd12e18a4b98d5c6a08db2cf54b03d3a65b Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 9 Oct 2026 23:23:53 -0400 Subject: [PATCH 5/5] docs(compass-app): name moved embedded symbols in comments Co-authored-by: Matt Wilkinson --- go/cmd/compass-app/embedded_launch_test.go | 2 +- go/cmd/compass-app/main.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/compass-app/embedded_launch_test.go b/go/cmd/compass-app/embedded_launch_test.go index c53bb02bf..8833b1e6f 100644 --- a/go/cmd/compass-app/embedded_launch_test.go +++ b/go/cmd/compass-app/embedded_launch_test.go @@ -129,7 +129,7 @@ func TestRunEmbeddedPreflightShortCircuits(t *testing.T) { // TestRunEmbeddedStackUpFails: a non-zero compass-stack up exit is surfaced and // the pipeline stops before WhoAmI. The stackUp seam already folds stderr into -// its error (see TestRunStackUpNonZeroExitSurfacesStderr); here the contract is +// its error (see embedded.TestRunStackUpNonZeroExitSurfacesStderr); here the contract is // that runEmbedded propagates it and does not dial. func TestRunEmbeddedStackUpFails(t *testing.T) { rec := &recorder{} diff --git a/go/cmd/compass-app/main.go b/go/cmd/compass-app/main.go index 637c7cd16..b63c41785 100644 --- a/go/cmd/compass-app/main.go +++ b/go/cmd/compass-app/main.go @@ -392,7 +392,7 @@ func distDirForExecutable(exe string) string { // controller. It runs the pipeline (preflight → stack up → WhoAmI) and, on // success, returns the resolved caller account id together with a *quitController // wired to the injected stackDown seam (its quit func is wired to app.Quit by -// run() once the app exists). resolveStackBin and this controller are embedded +// run() once the app exists). embedded.ResolveStackBin and this controller are embedded // concerns only: a client-only install has no compass-stack binary and no stack // to stop, so neither may gate a client launch (design §T5.6). func runEmbedded(