[quality] make pkg/dashboard copilot SDK-helper probe tests hermetic

changelog.d/fixed-quality-sdk-helper-test-hermetic.md

@@ -0,0 +1 @@
+- Tests: the copilot SDK-helper probe tests in `pkg/dashboard` are now hermetic. `TestProbeCopilotModelsSDK_AbsentHelperYieldsSentinel` and `TestProbeCopilotModelsSDK_HelperNotInstalled` used to run whatever was installed at the production helper path, so on any host that ships `copilot-models.mjs` without stored copilot auth (every live agent host) the "absent helper" sentinel never fired and the whole `pkg/dashboard` suite went red with `Not authenticated`. The helper path is now a test-seam var (`setCopilotSDKHelperPathForTest`, mirroring the `knowledge.SetBaseDirForTest` convention) pointed at a temp path, and a new `TestProbeCopilotModelsSDK_FailingHelperIsNotAbsent` covers the previously untestable-on-CI third state — helper present but exiting nonzero (the #7365 case) — via a stub script instead of the real helper.

src/pkg/dashboard/cli_models.go

@@ -114,17 +114,6 @@ const (
 	// require a plausible editor identifier.
 	copilotEditorVersion = "vscode/1.99.0"
 
-	// copilotSDKHelperPath is where the image installs the Node helper that
-	// lists Copilot models through the official @github/copilot-sdk (source:
-	// bin/copilot-models.mjs, COPYed by src/Dockerfile). The SDK spawns the
-	// pinned copilot CLI as a JSON-RPC server, so the probe rides the CLI's
-	// own stored auth and TLS handling — auth configurations the raw-HTTP
-	// probe below cannot reach (verified live against copilot CLI 1.0.59 in a
-	// hive pod: 25 models via stored device-flow auth behind the egress
-	// proxy). Absent outside the image, in which case the SDK probe is
-	// skipped instantly.
-	copilotSDKHelperPath = "/usr/local/bin/copilot-models.mjs"
-
 	// copilotSDKNodeBinary runs the helper. The helper is plain-Node ESM; it
 	// is invoked explicitly (not via shebang) so no exec bit is needed.
 	copilotSDKNodeBinary = "node"
@@ -269,6 +258,31 @@ const (
 // it at hermetic servers and exercise fallback branches without network.
 var copilotUserEndpointURL = "https://api.github.com/copilot_internal/user"
 
+// copilotSDKHelperPath is where the image installs the Node helper that
+// lists Copilot models through the official @github/copilot-sdk (source:
+// bin/copilot-models.mjs, COPYed by src/Dockerfile). The SDK spawns the
+// pinned copilot CLI as a JSON-RPC server, so the probe rides the CLI's
+// own stored auth and TLS handling — auth configurations the raw-HTTP
+// probe below cannot reach (verified live against copilot CLI 1.0.59 in a
+// hive pod: 25 models via stored device-flow auth behind the egress
+// proxy). Absent outside the image, in which case the SDK probe is
+// skipped instantly. A var (not const) only so tests can point it at a
+// hermetic path regardless of what the host has installed; production
+// never reassigns it.
+var copilotSDKHelperPath = "/usr/local/bin/copilot-models.mjs"
+
+// setCopilotSDKHelperPathForTest repoints the SDK helper script path for the
+// lifetime of t, mirroring the knowledge.SetBaseDirForTest convention.
+func setCopilotSDKHelperPathForTest(t interface {
+	Helper()
+	Cleanup(func())
+}, path string) {
+	t.Helper()
+	old := copilotSDKHelperPath
+	copilotSDKHelperPath = path
+	t.Cleanup(func() { copilotSDKHelperPath = old })
+}
+
 // --- Static fallback lists (kept CURRENT — July 2026) ---
 
 // claudeStaticModels is the fallback offered when the Claude models probe

src/pkg/dashboard/cli_models_sdk_severity_test.go

@@ -15,6 +15,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -39,15 +42,45 @@ func TestCopilotSDKHelperAbsentIsASentinel(t *testing.T) {
 
 // The real exec path must produce the sentinel when the helper script is not
 // installed, otherwise the sentinel is dead code and every dev machine starts
-// emitting a WARN.
+// emitting a WARN. The helper path is repointed at a path that does not exist
+// so the assertion holds regardless of whether the host image ships the real
+// helper (on live agent hosts it exists but is unauthenticated, which used to
+// flip this test to a hard FAIL — the third state the old version, which ran
+// whatever was at the production path, never accounted for).
 func TestProbeCopilotModelsSDK_AbsentHelperYieldsSentinel(t *testing.T) {
-	if _, err := execCopilotSDKHelper(context.Background(), ""); err != nil {
-		if !errors.Is(err, errCopilotSDKHelperAbsent) {
-			t.Fatalf("helper absence did not yield the sentinel: %v", err)
-		}
-		return
+	setCopilotSDKHelperPathForTest(t, filepath.Join(t.TempDir(), "copilot-models.mjs"))
+	_, err := execCopilotSDKHelper(context.Background(), "")
+	if err == nil {
+		t.Fatal("exec of a nonexistent helper unexpectedly succeeded")
+	}
+	if !errors.Is(err, errCopilotSDKHelperAbsent) {
+		t.Fatalf("helper absence did not yield the sentinel: %v", err)
+	}
+}
+
+// The converse on the same exec path: a helper that EXISTS and fails (#7365 —
+// exits nonzero) must NOT match the absence sentinel, or the failure would be
+// logged at INFO and stay invisible. Uses a stub script so the assertion never
+// depends on the host's real helper or its auth state.
+func TestProbeCopilotModelsSDK_FailingHelperIsNotAbsent(t *testing.T) {
+	if _, err := exec.LookPath("node"); err != nil {
+		t.Skip("node not on PATH; cannot exercise the helper exec path")
+	}
+	stub := filepath.Join(t.TempDir(), "copilot-models.mjs")
+	if err := os.WriteFile(stub, []byte("console.error('stub failure'); process.exit(1);\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	setCopilotSDKHelperPathForTest(t, stub)
+	_, err := execCopilotSDKHelper(context.Background(), "")
+	if err == nil {
+		t.Fatal("failing stub helper unexpectedly succeeded")
+	}
+	if errors.Is(err, errCopilotSDKHelperAbsent) {
+		t.Fatalf("a real helper failure matched the absence sentinel: %v", err)
+	}
+	if !strings.Contains(err.Error(), "stub failure") {
+		t.Errorf("helper stderr not folded into the error: %v", err)
 	}
-	t.Skip("copilot SDK helper present on this machine; skipping absence check")
 }
 
 // The severity split itself, asserted on real emitted log records.

src/pkg/dashboard/cli_models_sdk_test.go

@@ -3,6 +3,7 @@ package dashboard
 import (
 	"context"
 	"errors"
+	"path/filepath"
 	"testing"
 )
 
@@ -145,13 +146,15 @@ func TestQueryCLIModels_SDKResultsFeedRetention(t *testing.T) {
 
 // TestProbeCopilotModelsSDK_HelperNotInstalled verifies the real exec path
 // degrades instantly (no hang, no panic) when the helper script is absent —
-// the situation on dev machines and CI runners.
+// the situation on dev machines and CI runners. The helper path is repointed
+// at a nonexistent file so the check is hermetic on hosts that DO ship the
+// helper (where the old version exec'd the real helper — a network-dependent
+// probe whose outcome tracked the host's copilot auth state).
 func TestProbeCopilotModelsSDK_HelperNotInstalled(t *testing.T) {
+	setCopilotSDKHelperPathForTest(t, filepath.Join(t.TempDir(), "copilot-models.mjs"))
 	s := &Server{logger: testLogger()}
 	if _, err := s.probeCopilotModelsSDK(""); err == nil {
-		// The helper is installed only inside the hive image; if this machine
-		// actually has it, the probe may legitimately succeed — skip then.
-		t.Skip("copilot SDK helper present on this machine; skipping absence check")
+		t.Fatal("probe with an absent helper unexpectedly succeeded")
 	}
 }