Merge origin/main into agent/cmdcore-phase2

Resolve macOS CI auth/keychain job conflicts by keeping main's focused
6m keychain/auth split and unsigned-darwin self-heal filter, while retaining
this branch's portable DPAPI export/import diagnostics coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>

.github/workflows/ci.yml

@@ -702,14 +702,14 @@ jobs:
         with:
           go-version-file: go.mod
 
-      # Full ./internal/app (including schema --all assembly) already runs in the
-      # race:app shard. Keep this job focused on native auth/keychain paths so
-      # heavy Schema completeness tests cannot exhaust the 10m budget.
-      - name: Test macOS auth and Keychain paths with Race Detection
-        run: go test -v -race -count=1 -timeout=10m ./internal/keychain ./internal/auth
+      # Full app-package race (including schema --all assembly) already runs in
+      # the race:app shard. Keep this job focused on native auth/keychain paths
+      # so heavy Schema completeness tests cannot exhaust the budget.
+      - name: Test macOS auth and Keychain packages with Race Detection
+        run: go test -v -race -count=1 -timeout=6m ./internal/keychain ./internal/auth
 
-      - name: Test macOS auth migration and portable auth diagnostics
-        run: go test -v -race -count=1 -timeout=5m ./internal/app -run '^Test(CrossPlatformCoverage)?Auth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch|ExportRejectsWindowsDPAPIBackend|ImportRejectsWindowsDPAPIBackend)'
+      - name: Test macOS auth migration, Keychain diagnostics, and upgrade self-heal with Race Detection
+        run: go test -v -race -count=1 -timeout=5m ./internal/app -run '^(TestValidateNewBinary_RecoversFromUnsignedDarwin|Test(CrossPlatformCoverage)?Auth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch|ExportRejectsWindowsDPAPIBackend|ImportRejectsWindowsDPAPIBackend))'
 
   test-windows:
     name: Test (Windows)

internal/app/upgrade_test.go

@@ -543,6 +543,13 @@ func TestIsLikelyAMFIKill(t *testing.T) {
 // validateNewBinary recovers via repairDarwinBinary (ad-hoc codesign) and
 // successfully re-executes the binary. We use go itself as a stand-in for the
 // new dws binary — it's a real signed Mach-O we can strip and re-sign.
+//
+// GitHub's hosted macOS runners do not reproduce the amfid kill, so in CI this
+// test reports a skip that names the unverified path rather than implying the
+// self-heal was exercised. Set DWS_REQUIRE_AMFI_SELF_HEAL=1 on a host that does
+// enforce amfid to turn such a vacuous run into a hard failure.
+
+const requireAMFISelfHealEnv = "DWS_REQUIRE_AMFI_SELF_HEAL"
 
 func TestValidateNewBinary_RecoversFromUnsignedDarwin(t *testing.T) {
 	if runtime.GOOS != "darwin" {
@@ -573,15 +580,24 @@ func TestValidateNewBinary_RecoversFromUnsignedDarwin(t *testing.T) {
 		t.Fatalf("strip signature: %v\n%s", err, out)
 	}
 
-	// Sanity: confirm direct exec is killed.
+	// Sanity: confirm direct exec is killed. When it is not, repairDarwinBinary
+	// never runs and the rest of this test proves nothing — say so.
 	if _, err := tryExecVersion(bin); err == nil {
-		t.Skip("unsigned binary executed without amfid kill — likely Intel Mac or SIP disabled")
+		const unverified = "amfid did not kill the unsigned binary, so repairDarwinBinary was NOT exercised"
+		if os.Getenv(requireAMFISelfHealEnv) == "1" {
+			t.Fatalf("%s (%s=1)", unverified, requireAMFISelfHealEnv)
+		}
+		t.Skipf("%s — host does not enforce amfid (Intel Mac, SIP disabled, or hosted runner)", unverified)
 	}
 
 	// validateNewBinary should self-heal and succeed.
 	if err := validateNewBinary(bin, "dev"); err != nil {
 		if strings.Contains(err.Error(), "signal: killed") {
-			t.Skipf("host security policy still rejects the ad-hoc signed test binary: %v", err)
+			const unverified = "host security policy still rejects the ad-hoc signed binary, so the self-heal outcome was NOT verified"
+			if os.Getenv(requireAMFISelfHealEnv) == "1" {
+				t.Fatalf("%s (%s=1): %v", unverified, requireAMFISelfHealEnv, err)
+			}
+			t.Skipf("%s: %v", unverified, err)
 		}
 		t.Fatalf("validateNewBinary did not recover: %v", err)
 	}

test/scripts/changelog_pr_gate_test.go

@@ -553,19 +553,27 @@ func TestChangelogPRFastPathWorkflowContract(t *testing.T) {
 		t.Fatal("Code Admission workflow missing macOS test job boundaries")
 	}
 	darwinJob := admission[darwinStart:darwinEnd]
-	// Full ./internal/app race coverage lives in race:app; darwin stays focused on
-	// native auth/keychain plus a narrow auth-migration slice of internal/app.
+	// The macOS job no longer runs ./internal/app as a whole package, so it does
+	// not need the package-level race budget the Ubuntu shard gets — the Ubuntu
+	// "race: app" shard already covers everything except the natively-gated
+	// tests. Pinning the two focused commands replaces that budget assertion:
+	// it locks the per-step timeouts and blocks a whole-package regression.
+	// Keep portable DPAPI export/import diagnostics from this branch alongside
+	// main's unsigned-darwin self-heal coverage.
 	for _, want := range []string{
-		`go test -v -race -count=1 -timeout=10m ./internal/keychain ./internal/auth`,
-		`go test -v -race -count=1 -timeout=5m ./internal/app -run '^Test(CrossPlatformCoverage)?Auth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch|ExportRejectsWindowsDPAPIBackend|ImportRejectsWindowsDPAPIBackend)'`,
+		`go test -v -race -count=1 -timeout=6m ./internal/keychain ./internal/auth`,
+		`go test -v -race -count=1 -timeout=5m ./internal/app -run '^(TestValidateNewBinary_RecoversFromUnsignedDarwin|Test(CrossPlatformCoverage)?Auth(MigrateKeychain|StatusDiagnosticReportsCiphertextKeyMismatch|ExportRejectsWindowsDPAPIBackend|ImportRejectsWindowsDPAPIBackend))'`,
 	} {
 		if !strings.Contains(darwinJob, want) {
-			t.Errorf("macOS auth/keychain job missing contract %q", want)
+			t.Errorf("macOS native test job missing focused auth contract %q", want)
 		}
 	}
 	if strings.Contains(darwinJob, "./internal/keychain ./internal/auth ./internal/app") {
 		t.Error("macOS native test job must not repeat the complete internal/app race shard")
 	}
+	if count := strings.Count(darwinJob, "./internal/app"); count != 1 {
+		t.Errorf("macOS native test job internal/app invocation count = %d, want 1 focused invocation", count)
+	}
 
 	coverageStart := strings.Index(admission, "\n  coverage:\n")
 	coverageEnd := strings.Index(admission, "\n  policy:\n")

test/scripts/darwin_native_contract_test.go

@@ -0,0 +1,141 @@
+package scripts_test
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// TestMacOSNativeJobKeepsDarwinGatedAppTestsReachable couples the macOS test
+// job's -run pattern to the set of darwin-gated tests in internal/app.
+//
+// The Ubuntu race shard runs the whole package but skips anything gated on
+// runtime.GOOS != "darwin", and the platform coverage gate only runs
+// ^(TestAllShortcuts|TestCrossPlatformCoverage). That makes the macOS job the
+// only place a darwin-gated internal/app test can execute, so narrowing its
+// -run pattern can silently orphan one — which is exactly what happened in
+// #857 before review caught it.
+func TestMacOSNativeJobKeepsDarwinGatedAppTestsReachable(t *testing.T) {
+	root, err := filepath.Abs(filepath.Join("..", ".."))
+	if err != nil {
+		t.Fatalf("Abs(repo root) error = %v", err)
+	}
+
+	pattern := macOSAppRunPattern(t, root)
+	matcher, err := regexp.Compile(pattern)
+	if err != nil {
+		t.Fatalf("Compile(macOS -run pattern %q) error = %v", pattern, err)
+	}
+
+	gated := darwinGatedTestNames(t, filepath.Join(root, "internal", "app"))
+	if len(gated) == 0 {
+		t.Fatal("found no darwin-gated tests in internal/app; the scanner is broken or the gate style changed")
+	}
+
+	for _, name := range gated {
+		if !matcher.MatchString(name) {
+			t.Errorf(
+				"darwin-gated test %s is unreachable in CI: the Ubuntu shard skips it on Linux and the macOS -run pattern %q does not select it",
+				name,
+				pattern,
+			)
+		}
+	}
+}
+
+// macOSAppRunPattern extracts the -run pattern the macOS job applies to
+// ./internal/app from the CI workflow.
+func macOSAppRunPattern(t *testing.T, root string) string {
+	t.Helper()
+
+	data, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "ci.yml"))
+	if err != nil {
+		t.Fatalf("ReadFile(ci.yml) error = %v", err)
+	}
+	workflow := string(data)
+
+	start := strings.Index(workflow, "\n  test-darwin:\n")
+	end := strings.Index(workflow, "\n  test-windows:\n")
+	if start < 0 || end <= start {
+		t.Fatal("ci.yml missing macOS test job boundaries")
+	}
+
+	matches := regexp.MustCompile(`\./internal/app -run '([^']+)'`).FindAllStringSubmatch(workflow[start:end], -1)
+	if len(matches) != 1 {
+		t.Fatalf("macOS test job ./internal/app -run invocation count = %d, want exactly 1", len(matches))
+	}
+	return matches[0][1]
+}
+
+// darwinGatedTestNames returns every Test function in dir whose body gates on
+// runtime.GOOS != "darwin".
+func darwinGatedTestNames(t *testing.T, dir string) []string {
+	t.Helper()
+
+	entries, err := filepath.Glob(filepath.Join(dir, "*_test.go"))
+	if err != nil {
+		t.Fatalf("Glob(%s) error = %v", dir, err)
+	}
+
+	var names []string
+	fset := token.NewFileSet()
+	for _, entry := range entries {
+		file, parseErr := parser.ParseFile(fset, entry, nil, parser.SkipObjectResolution)
+		if parseErr != nil {
+			t.Fatalf("ParseFile(%s) error = %v", entry, parseErr)
+		}
+		for _, decl := range file.Decls {
+			fn, ok := decl.(*ast.FuncDecl)
+			if !ok || fn.Body == nil || !strings.HasPrefix(fn.Name.Name, "Test") {
+				continue
+			}
+			if gatesOnNonDarwin(fn.Body) {
+				names = append(names, fn.Name.Name)
+			}
+		}
+	}
+	return names
+}
+
+// gatesOnNonDarwin reports whether body contains a `runtime.GOOS != "darwin"`
+// comparison, the idiom this repo uses to skip a test off macOS.
+func gatesOnNonDarwin(body *ast.BlockStmt) bool {
+	found := false
+	ast.Inspect(body, func(node ast.Node) bool {
+		if found {
+			return false
+		}
+		binary, ok := node.(*ast.BinaryExpr)
+		if !ok || binary.Op != token.NEQ {
+			return true
+		}
+		if !isRuntimeGOOS(binary.X) {
+			return true
+		}
+		literal, ok := binary.Y.(*ast.BasicLit)
+		if !ok || literal.Kind != token.STRING {
+			return true
+		}
+		if literal.Value == `"darwin"` {
+			found = true
+			return false
+		}
+		return true
+	})
+	return found
+}
+
+// isRuntimeGOOS reports whether expr is the selector runtime.GOOS.
+func isRuntimeGOOS(expr ast.Expr) bool {
+	selector, ok := expr.(*ast.SelectorExpr)
+	if !ok || selector.Sel.Name != "GOOS" {
+		return false
+	}
+	ident, ok := selector.X.(*ast.Ident)
+	return ok && ident.Name == "runtime"
+}