trunk-merge/pr-1055/00a96fc0-bcff-48d5-b185-e92ef56a63d4

.github/workflows/ci.yml

@@ -1605,6 +1605,30 @@ jobs:
             echo "$store/bin" >>"$GITHUB_PATH"
           done
 
+      - name: Stack supervision unit tests (native darwin)
+        # The ONLY lane that EXECUTES the darwin process start-time identity
+        # reader. That reader is what lets `compass-stack up` run on macOS at
+        # all, and it is a two-site swap: the core records a token at spawn and
+        # the teardown adapter reads one back independently, with
+        # GroupSignaller.Alive comparing the two for uint64 equality. If the two
+        # darwin encodings ever drift, `down` matches nothing and SILENTLY skips
+        # every live child — no error, no signal, just an orphaned stack. Cross
+        # compiling from ubuntu type-checks those readers but never runs them,
+        # and the sysctl they call has no Linux equivalent to stand in, so this
+        # step is the only thing standing between a drifted encoding and a green
+        # PR. The suite is untagged: internal/stack is `//go:build unix`, which
+        # darwin satisfies, so a bare `go test` selects the _darwin.go readers
+        # and their mirrored packing tests with no `-tags` flag.
+        #
+        # It runs BEFORE the compile+bundle gate so an identity-reader
+        # regression reds fast rather than after the ~minutes-long bundle wrap.
+        # Deliberately NOT affected-guarded like that gate: this is seconds of
+        # pure-Go test on a runner the job already paid to boot, and the guard's
+        # own path list is the thing most likely to go stale.
+        env:
+          CGO_ENABLED: '0'
+        run: go -C go test -count=1 ./internal/stack/...
+
       - name: macOS compile + bundle gate
         # The ONE CI lane that compiles the native shell on darwin + exercises
         # the macos-bundle tool end to end (compass-distribution T3). It is a
@@ -1697,21 +1721,44 @@ jobs:
           # Run the darwin-tagged unit suite for the shell entrypoint — this is
           # the ONLY lane that executes it. The moon `compass-go:test` lane runs
           # untagged (`go test ./...`), which compiles the non-gtk4 stub and
-          # excludes main_test.go; the gtk4-e2e lane compiles the gtk4 build but
-          # `-run E2E`-filters, so it never executes TestDistDirForExecutable.
-          # That test defends the .app dist-resolution contract (the resolver
-          # returns Contents/Resources/dist under a Contents/MacOS executable,
-          # else dist beside it), which is exactly the behavioral change this
-          # lane ships — so its regression guard lives here or nowhere.
+          # excludes main_test.go, machine_test.go and embedded_test.go; the
+          # gtk4-e2e lane compiles the gtk4 build but `-run E2E`-filters, so it
+          # never reaches any of them. These tests defend contracts that ARE the
+          # darwin behaviour this lane ships, so their regression guards live
+          # here or nowhere:
+          #   - DistDirForExecutable: the .app dist-resolution contract
+          #     (Contents/Resources/dist under a Contents/MacOS executable,
+          #     else dist beside it).
+          #   - Machine*/EnsureMachine*: the podman-machine probe + ensure step,
+          #     including that an unclassifiable CLI answer provisions nothing.
+          #   - RealPreflightDeps*/ClassifyPreflight*: that the darwin machine
+          #     adapter is actually WIRED and that an unmet machine check is
+          #     fatal. This is the pair that catches the silent-skip regression
+          #     (a nil adapter making the check vanish into an all-green
+          #     preflight), so it is the last thing that should run nowhere.
+          #   - BringUpTimeout*: that darwin keeps a window a cold
+          #     `podman machine init` can fit inside.
           # `-run` alone exits 0 when it matches nothing (a rename → false
-          # green), so require the test's own PASS line — a rename or skip reds.
+          # 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.
           CGO_ENABLED=1 go -C go test -trimpath \
-            -run 'TestDistDirForExecutable' -count=1 -v \
+            -run 'TestDistDirForExecutable|TestMachineReady|TestEnsureMachineReady|TestMachineResourceFloorIsExplicit|TestRealPreflightDeps|TestClassifyPreflight|TestBringUpTimeout' \
+            -count=1 -v \
             ./cmd/compass-app/ | tee /tmp/darwin-unit.log
-          grep -q '^--- PASS: TestDistDirForExecutable' /tmp/darwin-unit.log || {
-            echo "::error::darwin: TestDistDirForExecutable did not run+pass (renamed or skipped?)"
-            exit 1
-          }
+          for t in TestDistDirForExecutable \
+                   TestMachineReadyRunning \
+                   TestMachineReadyNoMachine \
+                   TestEnsureMachineReadyNoMachineProvisions \
+                   TestEnsureMachineReadyUnclassifiedDoesNotProvision \
+                   TestRealPreflightDepsWiresDarwinMachineAdapter \
+                   TestClassifyPreflightUnwiredDarwinMachineIsFatal \
+                   TestBringUpTimeoutBudgetsDarwinColdProvisioning; do
+            grep -q "^--- PASS: $t" /tmp/darwin-unit.log || {
+              echo "::error::darwin: $t did not run+pass (renamed or skipped?)"
+              exit 1
+            }
+          done
 
           # The UI dist the .app stages into Contents/Resources/dist.
           moon run compass-ui:build

apps/ui/src/analytics/analytics.test.ts

@@ -14,6 +14,7 @@ interface FakePostHog {
 	capture: (...args: unknown[]) => void;
 	identify: (...args: unknown[]) => void;
 	reset: (...args: unknown[]) => void;
+	get_session_id: (...args: unknown[]) => string;
 }
 
 function makeFake(): FakePostHog {
@@ -29,6 +30,10 @@ function makeFake(): FakePostHog {
 		capture: record("capture"),
 		identify: record("identify"),
 		reset: record("reset"),
+		get_session_id: () => {
+			calls.push({ method: "get_session_id", args: [] });
+			return "";
+		},
 	};
 }
 
@@ -46,6 +51,38 @@ describe("createAnalytics", () => {
 		expect(fake.calls).toHaveLength(0);
 	});
 
+	test("disabled sessionId returns undefined with ZERO posthog calls", () => {
+		const fake = makeFake();
+		const analytics = createAnalytics(undefined, {
+			posthog: fake as unknown as PostHog,
+		});
+
+		expect(analytics.sessionId()).toBeUndefined();
+		expect(fake.calls).toHaveLength(0);
+	});
+
+	test("enabled sessionId delegates to posthog", () => {
+		const fake = makeFake();
+		fake.get_session_id = () => "session-123";
+		const analytics = createAnalytics(
+			{ key: "phc_abc", host: "https://us.i.posthog.com" },
+			{ posthog: fake as unknown as PostHog },
+		);
+
+		expect(analytics.sessionId()).toBe("session-123");
+	});
+
+	test("enabled sessionId maps an empty posthog id to undefined", () => {
+		const fake = makeFake();
+		fake.get_session_id = () => "";
+		const analytics = createAnalytics(
+			{ key: "phc_abc", host: "https://us.i.posthog.com" },
+			{ posthog: fake as unknown as PostHog },
+		);
+
+		expect(analytics.sessionId()).toBeUndefined();
+	});
+
 	test("disabled with NO deps (the production shape) is a callable no-op", () => {
 		// index.tsx calls createAnalytics(analyticsConfigFromEnv()) with no deps,
 		// so the disabled production path is createAnalytics(undefined) — the real
@@ -109,6 +146,30 @@ describe("createAnalytics", () => {
 		expect(identifies).toHaveLength(1);
 		expect(identifies[0]?.args).toEqual(["acct-1"]);
 	});
+
+	// The session id ROTATES under the app (posthog mints a new one on idle and
+	// at max length), so the value must be re-read per call and never memoized.
+	// The interceptor side is pinned for this too, but a memo added HERE would
+	// defeat that: the interceptor would faithfully re-read a stale cache.
+	test("enabled sessionId re-reads posthog on every call, never memoizing", () => {
+		const fake = makeFake();
+		const ids = ["sess-1", "sess-2"];
+		let call = 0;
+		fake.get_session_id = () => {
+			fake.calls.push({ method: "get_session_id", args: [] });
+			return ids[call++] ?? "";
+		};
+		const analytics = createAnalytics(
+			{ key: "phc_abc", host: "https://us.i.posthog.com" },
+			{ posthog: fake as unknown as PostHog },
+		);
+
+		expect(analytics.sessionId()).toBe("sess-1");
+		expect(analytics.sessionId()).toBe("sess-2");
+		expect(
+			fake.calls.filter((c) => c.method === "get_session_id"),
+		).toHaveLength(2);
+	});
 });
 
 describe("$ai_trace_id stamping", () => {

apps/ui/src/analytics/analytics.ts

@@ -27,6 +27,8 @@ export interface Analytics {
 	capture(event: string, props?: Record<string, unknown>): void;
 	/** Associate subsequent events with a stable distinct id (the caller). */
 	identify(distinctId: string): void;
+	/** Return the current PostHog session id, when one exists. */
+	sessionId(): string | undefined;
 	/** Tear down the identified session (logout / app teardown). */
 	shutdown(): void;
 }
@@ -36,6 +38,9 @@ export interface Analytics {
 class NoopAnalytics implements Analytics {
 	capture(): void {}
 	identify(): void {}
+	sessionId(): string | undefined {
+		return undefined;
+	}
 	shutdown(): void {}
 }
 
@@ -45,8 +50,10 @@ class NoopAnalytics implements Analytics {
 class PostHogAnalytics implements Analytics {
 	private readonly client: PostHog;
 	/** The trace-id source, read at CAPTURE time rather than construction time:
-	 *  the transport that records trace ids is built before this client exists,
-	 *  so a value read once at construction would always be undefined.
+	 *  boot builds analytics BEFORE the transport, so this getter closes over a
+	 *  `clients` binding that is not yet initialized — reading it at construction
+	 *  would throw a ReferenceError, while reading it at capture time is long
+	 *  after boot bound it.
 	 *
 	 *  A getter, not the sink object, on purpose — analytics reads one string and
 	 *  has no business depending on compass-client's transport types, so the
@@ -106,6 +113,11 @@ class PostHogAnalytics implements Analytics {
 		this.client.identify(distinctId);
 	}
 
+	sessionId(): string | undefined {
+		const sessionId = this.client.get_session_id();
+		return sessionId === "" ? undefined : sessionId;
+	}
+
 	shutdown(): void {
 		// PostHog's de-identify: reset the distinct id and start a fresh
 		// anonymous session. The browser SDK batch-sends on its own; there is no

apps/ui/src/index.tsx

@@ -88,39 +88,53 @@ async function main(
 	root: HTMLElement,
 	connection: ResolvedConnection,
 ): Promise<void> {
-	const clients = createLiveClients(connection);
-
-	const callerId = await bootCaller(root, () => resolveCaller(clients.compass));
-	// Undefined is bootCaller's stop signal — it already painted the WhoAmI
-	// failure screen, so the app must not come up (no caller to scope it).
-	if (!callerId) {
-		return;
-	}
-
 	// Product analytics, OFF by default: analyticsConfigFromEnv returns undefined
 	// unless a PostHog project key is configured, and createAnalytics then hands
 	// back a no-op that never touches posthog — an unconfigured deployment emits
-	// zero analytics. Identify the caller we just learned via WhoAmI so events
-	// attach to a stable distinct id.
+	// zero analytics.
 	//
-	// The inbound half of correlation is wired here: `clients.traceId` is the slot
-	// the transport records each reply's server trace id into, and analytics reads
-	// it at capture time. Reading through a getter is what makes the ordering work
-	// — the clients exist before this line, but the first trace id only lands once
-	// a call has returned.
+	// Built FIRST, before the clients, because correlation now runs in both
+	// directions and the outbound half needs a real analytics object to read
+	// from. Both directions are lazy getters, and they point opposite ways:
 	//
-	// Best-effort by construction, on two counts. The slot holds the LAST reply's
-	// trace id, so an event fired before any call has returned carries nothing,
-	// and one fired between calls carries the previous call's trace rather than
-	// its own. And the server sets `traceresponse` only on UNARY replies, and
-	// only when an OTel provider is installed — an unconfigured deployment
-	// (empty exporter endpoint ⇒ no span ⇒ no header) stamps nothing at all.
+	//   inbound   `clients.traceId` → analytics: the transport records each
+	//             reply's server trace id into that slot, and analytics reads it
+	//             at capture time. `clients` is a forward reference from inside
+	//             this getter, which is safe because the getter only runs once
+	//             an event is captured — long after the next statement binds it.
+	//   outbound  `analytics.sessionId()` → the transport: every request asks
+	//             for the current PostHog session id and sends it as
+	//             X-POSTHOG-SESSION-ID, so backend spans carry the same session
+	//             the frontend recorded.
 	//
-	// The OUTBOUND half (sending the PostHog session id to the server so its
-	// spans carry it) is deliberately not wired here.
+	// The inbound half is best-effort by construction, on two counts. The slot
+	// holds the LAST reply's trace id, so an event fired before any call has
+	// returned carries nothing, and one fired between calls carries the previous
+	// call's trace rather than its own. And the server sets `traceresponse` only
+	// on UNARY replies, and only when an OTel provider is installed — an
+	// unconfigured deployment (empty exporter endpoint ⇒ no span ⇒ no header)
+	// stamps nothing at all.
+	//
+	// The outbound half is best-effort too: the getter returns undefined until a
+	// PostHog session exists, and the interceptor then sends no header and
+	// self-heals on the next request. Only the TLS network door reads the header.
 	const analytics = createAnalytics(analyticsConfigFromEnv(), {
 		traceId: () => clients.traceId.current,
 	});
+
+	const clients = createLiveClients(connection, {
+		sessionId: () => analytics.sessionId(),
+	});
+
+	const callerId = await bootCaller(root, () => resolveCaller(clients.compass));
+	// Undefined is bootCaller's stop signal — it already painted the WhoAmI
+	// failure screen, so the app must not come up (no caller to scope it).
+	if (!callerId) {
+		return;
+	}
+
+	// Identify the caller we just learned via WhoAmI so events attach to a stable
+	// distinct id. This stays AFTER bootCaller: the id is its output.
 	analytics.identify(callerId);
 
 	// One app-lifetime QueryClient — the server-state cache the query layer keys

apps/ui/src/live/client.test.ts

@@ -43,6 +43,27 @@ describe("createLiveClients (query record T1)", () => {
 		expect(compassTransport).toBe(clients.transport);
 		expect(commsTransport).toBe(compassTransport);
 	});
+
+	test("passes the session id getter through to the transport factory", () => {
+		const transportSpy = spyOn(compassClient, "createCompassWebTransport");
+		spies.push(transportSpy);
+		const sessionId = () => "session-id";
+
+		createLiveClients(conn, { sessionId });
+
+		const opts = transportSpy.mock.calls[0]?.[2];
+		expect(opts?.sessionId).toBe(sessionId);
+	});
+
+	test("omits the session id option when deps are omitted", () => {
+		const transportSpy = spyOn(compassClient, "createCompassWebTransport");
+		spies.push(transportSpy);
+
+		createLiveClients(conn);
+
+		const opts = transportSpy.mock.calls[0]?.[2];
+		expect(opts).not.toHaveProperty("sessionId");
+	});
 });
 
 describe("resolveCaller (WhoAmI boot probe)", () => {

apps/ui/src/live/client.ts

@@ -41,9 +41,9 @@ export interface LiveClients {
 	 *
 	 *  It rides on LiveClients because this is the one place that owns transport
 	 *  construction — the sink is WRITTEN by the transport layer and READ above
-	 *  it (analytics stamps it on captured events), and boot builds the clients
-	 *  before analytics exists, so a shared mutable slot handed out here is what
-	 *  connects a writer and a reader that can never meet at construction. */
+	 *  it (analytics stamps it on captured events). A shared mutable slot is what
+	 *  connects them because the trace id arrives on a REPLY: the writer has no
+	 *  value to hand over at construction time, whatever order boot runs in. */
 	readonly traceId: TraceIdSink;
 }
 
@@ -54,11 +54,15 @@ export interface LiveClients {
  *  cache-coherent with the clients' calls. `conn.fetchImpl` threads the resolved
  *  transport fetch through: undefined (browser dev) uses the platform fetch; a
  *  shell-provided fetch tunnels over IPC — the seam is invisible above here. */
-export function createLiveClients(conn: ResolvedConnection): LiveClients {
+export function createLiveClients(
+	conn: ResolvedConnection,
+	deps?: { sessionId?: () => string | undefined },
+): LiveClients {
 	const traceId: TraceIdSink = { current: undefined };
 	const transport = createCompassWebTransport(conn.baseUrl, conn.token, {
 		fetch: conn.fetchImpl,
 		traceSink: traceId,
+		...(deps?.sessionId === undefined ? {} : { sessionId: deps.sessionId }),
 	});
 	return {
 		comms: createCommsClient(transport),