fix(tmux): deliver session env via set-environment, not -e argv (ADR-0051)

TESTING.md

@@ -451,7 +451,7 @@ all-source audit while staying outside untagged and Small debt.
 <!-- BEGIN CHECKED TEST RESOURCE LEDGER -->
 | Ledger kind | Source scope | Resource baseline | Tracking owner | Invariant / resource owner | Migration | Expiry |
 | --- | --- | --- | --- | --- | --- | --- |
-| Audit baseline | all tracked test source | fixed_sleep: 437 calls / 161 files (historical regex census: 447 / 157) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 |
+| Audit baseline | all tracked test source | fixed_sleep: 438 calls / 161 files (historical regex census: 447 / 157) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 |
 | Audit baseline | all tracked test source | listener_helper: 58 calls / 23 files | ga-80po0c.2.2.3 | all-source listener-helper call/file totals cannot drift without an explicit checked policy update; ga-80po0c.2.2.3 owns this all-source audit; tagged calls stay Large and receive no Medium exemption | P0.4c-listener-helper | 2026-10-01 |
 | Audit baseline | all tracked test source | subprocess: 555 calls / 168 files (historical regex census: 495 / 135) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 |
 | Medium owner | `cmd/gc` package `main` | TestMain: environment, tmux | ga-80po0c.2.1 | cmd/gc TestMain is the checked package-level Medium owner for process environment and tmux namespace setup; only declared environment and tmux calls lexically inside TestMain leave Small debt | P0.4b/P0.4c-tmux | 2026-10-01 |

internal/runtime/tmux/agent_slice_test.go

@@ -53,18 +53,47 @@ func TestAgentSliceWrapsNewSessionWithCommandAndEnv(t *testing.T) {
 	if len(exec.calls) == 0 {
 		t.Fatal("no tmux calls recorded")
 	}
-	args := exec.calls[0]
-	got := args[len(args)-1]
-	// The env -u prefix must end up INSIDE the scope wrapper so the unset
-	// still applies to the agent process.
-	want := "systemd-run --user --scope --slice=gascity-agents.slice --collect --quiet -- sh -c 'env -u LC_ALL claude'"
-	if got != want {
-		t.Fatalf("pane command = %q, want %q", got, want)
+
+	// ADR-0051 transport: no -e flags anywhere (secrets would pin to argv), and
+	// the agent command is started via respawn-pane, which wrapPaneCommand still
+	// wraps in the systemd scope. Empty-value vars are removed via set-environment
+	// -r, NOT via an env -u prefix on the command — so the wrapped command is
+	// the plain agent command.
+	for i, call := range exec.calls {
+		for _, a := range call {
+			if a == "-e" {
+				t.Fatalf("ADR-0051: call %d still uses -e flag: %v", i, call)
+			}
+		}
+	}
+
+	// LANG is delivered via set-environment over the socket.
+	foundLANG := false
+	for _, call := range exec.calls {
+		if sub := setEnvironmentArgs(call); contains(sub, "LANG") && contains(sub, "en_US.UTF-8") {
+			foundLANG = true
+		}
+	}
+	if !foundLANG {
+		t.Errorf("missing set-environment LANG en_US.UTF-8; calls: %v", exec.calls)
 	}
-	// The -e session env flags must survive wrapping.
-	joined := strings.Join(args, "\x00")
-	if !strings.Contains(joined, "\x00-e\x00LANG=en_US.UTF-8\x00") {
-		t.Fatalf("new-session args missing LANG -e flag: %v", args)
+	assertEnvRemovedForNewProcess(t, exec.calls, "LC_ALL")
+
+	// The command is started via respawn-pane and STILL wrapped in the systemd
+	// scope (wrapPaneCommand applies inside RespawnPane).
+	var respawnCmd string
+	for _, call := range exec.calls {
+		if cmd(call) == "respawn-pane" {
+			respawnCmd = call[len(call)-1]
+			break
+		}
+	}
+	want := "systemd-run --user --scope --slice=gascity-agents.slice --collect --quiet -- sh -c claude"
+	if respawnCmd == "" {
+		t.Fatalf("missing respawn-pane call; calls: %v", exec.calls)
+	}
+	if respawnCmd != want {
+		t.Fatalf("respawn pane command = %q, want %q", respawnCmd, want)
 	}
 }
 
@@ -154,14 +183,51 @@ func TestAgentSliceEmptyCommandNotWrapped(t *testing.T) {
 	t.Setenv(AgentSliceEnv, "gascity-agents.slice")
 	tm, exec := newSliceTestTmux(t)
 
-	// Empty command + env-only session must keep the empty trailing arg so
-	// tmux still starts the default shell.
+	// Empty command + env-only session: a bare new-session (no command) starts
+	// tmux's default shell, env is set via set-environment, and the pane is then
+	// respawned with NO shell-command so the shell restarts carrying that env.
+	//
+	// The respawn is required, not optional: the shell new-session started
+	// captured its environment before set-environment ran, so skipping the
+	// respawn delivers nothing to the pane's process — while show-environment
+	// still reports every variable as set. Because no shell-command is passed,
+	// wrapPaneCommand does not apply and the systemd wrapper must not appear.
 	if err := tm.NewSessionWithCommandAndEnv("gc-test-empty", "/work", "", map[string]string{"LANG": "C"}); err != nil {
 		t.Fatalf("NewSessionWithCommandAndEnv: %v", err)
 	}
-	args := exec.calls[0]
-	if got := args[len(args)-1]; got != "" {
-		t.Fatalf("pane command = %q, want empty", got)
+	for i, call := range exec.calls {
+		if c := call[len(call)-1]; strings.Contains(c, "systemd-run") {
+			t.Fatalf("empty command should not be systemd-wrapped; call %d: %v", i, call)
+		}
+	}
+	var respawn []string
+	for _, call := range exec.calls {
+		if cmd(call) == "respawn-pane" {
+			respawn = call
+			break
+		}
+	}
+	if respawn == nil {
+		t.Fatalf("env-only session must respawn the pane so the shell picks up the "+
+			"session env; calls: %v", exec.calls)
+	}
+	// respawn-pane -k -t <session>, and nothing after the target: a trailing
+	// shell-command would be the wrapped form this test exists to exclude.
+	if got, want := respawn[len(respawn)-1], "gc-test-empty"; got != want {
+		t.Fatalf("env-only respawn should carry no shell-command; last arg = %q, want %q (call: %v)",
+			got, want, respawn)
+	}
+	// LANG still delivered via set-environment. Shape only — the behavioral
+	// counterpart is TestNewSessionWithCommandAndEnvDeliversEnvToEnvOnlyShell.
+	delivered := false
+	for _, call := range exec.calls {
+		if sub := setEnvironmentArgs(call); contains(sub, "LANG") && contains(sub, "C") {
+			delivered = true
+			break
+		}
+	}
+	if !delivered {
+		t.Fatalf("missing set-environment LANG C; calls: %v", exec.calls)
 	}
 }
 

internal/runtime/tmux/executor_test.go

@@ -3,6 +3,7 @@ package tmux
 import (
 	"context"
 	"errors"
+	"slices"
 	"strconv"
 	"strings"
 	"testing"
@@ -60,13 +61,221 @@ func TestNewSessionWithCommandAndEnvClearsEmptyVars(t *testing.T) {
 		t.Fatal("no tmux calls recorded")
 	}
 
-	args := exec.calls[0]
-	joined := strings.Join(args, "\x00")
-	if !strings.Contains(joined, "\x00-e\x00LANG=en_US.UTF-8\x00") {
-		t.Fatalf("new-session args missing LANG -e flag: %v", args)
+	// C2 (ADR-0051): no -e flag anywhere in the launch sequence — that is the
+	// argv-exposure defect this change removes. (runCtx prepends -u/-L to every
+	// call, so scan every arg of every call.)
+	for i, args := range exec.calls {
+		for _, a := range args {
+			if a == "-e" {
+				t.Fatalf("call %d still uses -e flag (ADR-0051 transport): %v", i, args)
+			}
+		}
+	}
+
+	// The session is created bare (no command, no -e). cmd() finds the tmux
+	// subcommand token past the -u/-L prefix injected by runCtx.
+	newSession := exec.calls[0]
+	if cmd(newSession) != "new-session" {
+		t.Fatalf("first call = %q, want new-session: %v", cmd(newSession), newSession)
+	}
+	for _, a := range newSession {
+		if a == "claude" || strings.HasPrefix(a, "env ") {
+			t.Fatalf("new-session should not carry the command: %v", newSession)
+		}
+	}
+
+	// LANG is set over the socket via set-environment; empty values via -r.
+	//
+	// The flag must be -r, not -u: -u drops only the session-scope entry, after
+	// which tmux re-merges the server-global environment when respawn-pane starts
+	// the command — so a stale global LC_ALL/LC_CTYPE would reach the pane anyway.
+	// -r is "removed from the environment before starting a new process".
+	// NOTE: this is a call-SHAPE assertion and cannot observe the actual child
+	// env; TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess is
+	// the behavioral counterpart that fails if this flag regresses to -u.
+	foundSet := false
+	for _, args := range exec.calls {
+		// set-environment -t <session> KEY VALUE
+		if sub := setEnvironmentArgs(args); contains(sub, "LANG") && contains(sub, "en_US.UTF-8") {
+			foundSet = true
+		}
+	}
+	if !foundSet {
+		t.Errorf("missing set-environment LANG en_US.UTF-8")
+	}
+	assertEnvRemovedForNewProcess(t, exec.calls, "LC_ALL")
+	assertEnvRemovedForNewProcess(t, exec.calls, "LC_CTYPE")
+
+	// The command is started via respawn-pane -k -t <session> <wrapped command>.
+	var respawn []string
+	for _, args := range exec.calls {
+		if cmd(args) == "respawn-pane" {
+			respawn = args
+			break
+		}
+	}
+	if respawn == nil {
+		t.Fatal("missing respawn-pane call to start the command")
+	}
+	if got := respawn[len(respawn)-1]; !strings.Contains(got, "claude") {
+		t.Fatalf("respawn-pane command = %q, want it to contain claude", got)
+	}
+	// The env -u prefix the old transport bolted onto the command is gone —
+	// unsetting is now a session-level set-environment -r, not a command prefix.
+	if got := respawn[len(respawn)-1]; strings.HasPrefix(got, "env ") {
+		t.Fatalf("respawn-pane command should not carry an env -u prefix: %q", got)
+	}
+}
+
+// contains reports whether args contains s.
+func contains(args []string, s string) bool {
+	for _, a := range args {
+		if a == s {
+			return true
+		}
+	}
+	return false
+}
+
+// cmd returns the tmux subcommand token from a recorded call, skipping the -u
+// and -L <socket> flags that runCtx prepends to every invocation.
+func cmd(args []string) string {
+	for i := 0; i < len(args); i++ {
+		switch args[i] {
+		case "-u":
+			continue
+		case "-L":
+			i++ // skip socket name
+			continue
+		default:
+			return args[i]
+		}
+	}
+	return ""
+}
+
+// setEnvironmentArgs returns the arguments a recorded call passed to its own
+// set-environment subcommand, or nil when the call is not set-environment.
+//
+// Every set-environment flag assertion must go through this. runCtx prepends
+// tmux's global -u (force UTF-8) to EVERY invocation, so scanning the whole argv
+// for "-u" matches that wrapper flag and stays true even when set-environment
+// was never given it. That vacuity is what let the original -u/-r defect ship
+// green; repairing it in one test file while an identical copy survived in
+// another is how it stayed green afterwards. The scan lives in one place so a
+// fix cannot land in only half the call sites.
+func setEnvironmentArgs(args []string) []string {
+	if cmd(args) != "set-environment" {
+		return nil
+	}
+	return args[slices.Index(args, "set-environment")+1:]
+}
+
+// assertEnvRemovedForNewProcess fails unless the recorded calls remove key with
+// set-environment -r, and fails if any of them removes it with -u instead.
+//
+// -u drops only the session-scope entry; tmux re-merges the server-global
+// environment when respawn-pane starts the process, so a stale global value
+// comes back. -r is "removed from the environment before starting a new
+// process". This is a call-SHAPE assertion — the behavioral counterpart is
+// TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess.
+func assertEnvRemovedForNewProcess(t *testing.T, calls [][]string, key string) {
+	t.Helper()
+	removed := false
+	for _, args := range calls {
+		sub := setEnvironmentArgs(args)
+		if sub == nil || !contains(sub, key) {
+			continue
+		}
+		if contains(sub, "-u") {
+			t.Errorf("launch path used set-environment -u %s: -u drops only the "+
+				"session-scope entry, so tmux re-merges the server-global value when "+
+				"respawn-pane starts the process; want -r; call: %v", key, args)
+		}
+		if contains(sub, "-r") {
+			removed = true
+		}
+	}
+	if !removed {
+		t.Errorf("missing set-environment -r %s (empty-value removal); calls: %v", key, calls)
+	}
+}
+
+// TestNewSessionWithCommandAndEnvNoSecretInArgv is the load-bearing ADR-0051
+// regression (Acceptance criterion C2): no secret value may be pinned to a
+// long-lived process's argv. The -e transport placed every secret on the
+// new-session command, which becomes the persistent tmux *server* argv; the
+// set-environment transport must not reintroduce that.
+//
+// C2 scope note (ADR-0051 "C2 scope correction"): set-environment takes the value
+// as a positional argv argument of a short-lived tmux *client* that exits in
+// milliseconds. That transient client argv is the acknowledged bounded residual —
+// NOT what this test guards. This test guards the persistent surface: the
+// new-session call (server argv) and the respawn-pane call (the long-lived pane
+// process). It also asserts no -e flag exists anywhere in the launch sequence.
+func TestNewSessionWithCommandAndEnvNoSecretInArgv(t *testing.T) {
+	exec := &fakeExecutor{}
+	tm := NewTmux()
+	tm.exec = exec
+
+	const secretValue = "sk-SECRET-v1-0123456789-do-not-leak"
+	env := map[string]string{
+		"ANTHROPIC_AUTH_TOKEN": secretValue,
+		"OPENROUTER_API_KEY":   secretValue,
+		"GC_INSTANCE_TOKEN":    secretValue,
+		"BEADS_HOLDER_TOKEN":   secretValue, // alias of GC_INSTANCE_TOKEN (same value, different name)
+		"GT_ROLE":              "testrig/crew/x",
+	}
+	if err := tm.NewSessionWithCommandAndEnv("gc-test-no-leak", "", "claude", env); err != nil {
+		t.Fatalf("NewSessionWithCommandAndEnv: %v", err)
+	}
+	if len(exec.calls) == 0 {
+		t.Fatal("no tmux calls recorded")
+	}
+
+	// 1. No -e flag anywhere in the launch sequence (transport is clean).
+	for i, args := range exec.calls {
+		for _, a := range args {
+			if a == "-e" {
+				t.Fatalf("ADR-0051 C2 violation: call %d (%s) still uses -e flag: %v", i, args[0], args)
+			}
+		}
+	}
+
+	// 2. No secret value in the PERSISTENT surfaces: new-session (server argv)
+	//    and respawn-pane (the long-lived pane process). set-environment calls are
+	//    the transient client and are intentionally excluded.
+	persistent := []string{"new-session", "respawn-pane"}
+	for i, args := range exec.calls {
+		if !contains(persistent, cmd(args)) {
+			continue
+		}
+		joined := strings.Join(args, "\x00")
+		if strings.Contains(joined, secretValue) {
+			t.Fatalf("ADR-0051 C2 violation: secret value leaked into persistent %s "+
+				"argv (call %d): %v", cmd(args), i, args)
+		}
+		// Even a "KEY=VALUE" pair (the -e serialization shape) must not appear.
+		if strings.Contains(joined, "ANTHROPIC_AUTH_TOKEN=") ||
+			strings.Contains(joined, "OPENROUTER_API_KEY=") ||
+			strings.Contains(joined, "GC_INSTANCE_TOKEN=") ||
+			strings.Contains(joined, "BEADS_HOLDER_TOKEN=") {
+			t.Fatalf("ADR-0051 C2 violation: KEY=VALUE pair in persistent %s argv "+
+				"(call %d): %v", cmd(args), i, args)
+		}
+	}
+
+	// 3. Sanity: env WAS delivered — via set-environment (not -e). At least one
+	//    set-environment call carries the GT_ROLE value (non-secret).
+	delivered := false
+	for _, args := range exec.calls {
+		if cmd(args) == "set-environment" && contains(args, "testrig/crew/x") {
+			delivered = true
+			break
+		}
 	}
-	if got := args[len(args)-1]; got != "env -u LC_ALL -u LC_CTYPE claude" {
-		t.Fatalf("command = %q, want env -u LC_ALL -u LC_CTYPE claude", got)
+	if !delivered {
+		t.Errorf("expected a set-environment call delivering GT_ROLE; calls: %v", exec.calls)
 	}
 }