refactor(engine): collapse resolver host I/O onto the Session seam (deepen-engine-seams)

CHANGELOG.md

@@ -32,6 +32,19 @@
   a native `rc` release gate, and CI cross-compile coverage. Plan 9 release
   artifacts remain a separate promotion decision.
 
+### Changed
+
+- Internal, behavior-preserving refactor of the discovery engine: fact
+  resolvers now reach the host only through the run-scoped Session seam, with
+  an automated check freezing that boundary. Category assembly reads platform
+  identity and environment through the Session so windows/plan9 paths are
+  exercisable with a fake host; the host-virtualization signals are gathered
+  once per discovery instead of up to three times; cloud metadata transport is
+  consolidated in one helper; and dead/test-only entrances (`detector.go`, the
+  `query.go` Select delegates, the `LoadExternalFacts` facade, `filehelper.go`,
+  the version fast path's crutch exports) are removed. No public API, CLI flag,
+  output, input-source precedence, diagnostic, or cache behavior changes.
+
 ### Fixed
 
 - The DragonFly `disks`/`partitions` probe no longer reports empty memory-disk

docs/adr/0010-core-facts-split-by-category.md

@@ -11,7 +11,7 @@ We adopt **per-fact-category resolver modules** as the standard organization for
 Two constraints that must not be re-crossed:
 
 - **Platform splits within a category must not use Go's reserved GOOS suffixes.** A file named `networking_windows.go`, `*_linux.go`, `*_darwin.go`, or `*_freebsd.go` is given an *implicit* build constraint by the Go toolchain and compiles only on that OS. The Windows networking *parsing* logic is deliberately tested on Linux/macOS CI through the `goos`-string parameter seam (AGENTS.md: "platform logic should be tested through fixtures or injected probes"); a GOOS-suffixed file would exclude that logic from other platforms' builds and tests, silently breaking the seam. When a category file grows unwieldy and a hybrid by-platform split is warranted, use a non-reserved name (e.g. `networking_msft.go`), never a GOOS suffix. Genuine syscall-bound code that *should* be GOOS-constrained already lives in its own tagged files (`statfs_linux.go`); this convention is about the cross-platform resolver/parse logic.
-- **This split does not change function signatures.** Collapsing the `commandRunner`/`fileReader` parameters into `Session` methods is a separate, already-deferred follow-on (recorded in the 2026-06-17 deepen-engine-internals design). The category split moves functions and adds per-category assembly funcs; it does not rewrite their parameters, so the two changes stay independently reviewable.
+- **This split does not change function signatures.** Collapsing the `commandRunner`/`fileReader` parameters into `Session` methods is a separate, already-deferred follow-on (recorded in the 2026-06-17 deepen-engine-internals design). The category split moves functions and adds per-category assembly funcs; it does not rewrite their parameters, so the two changes stay independently reviewable. **Done (deepen-engine-seams, 2026-07):** the resolver `commandRunner`/`fileReader` threading and the `FromRoot`/`WithHost`/`WithReader`/`ForPlatform` variant families are collapsed onto the Session host seam, which is now the only resolver host-I/O path — structurally enforced by `TestNoRawHostIOInResolvers`. Pure `parse*`/goos-string-parameter signatures are unchanged, as this ADR requires; the recorded accepted leaks (`exec.LookPath` in the Linux distro probe, identity's uid/gid syscalls, the uptime clock, `net.Interfaces`) remain injectable parameters. That change also resolved the archived 2026-06-17 open question that had marked the `LoadExternalFacts` facade for deletion.
 
 ## Considered Options
 

internal/app/app.go

@@ -297,13 +297,11 @@ func runQuery(stdout, stderr io.Writer, args []string) error {
 		disabledFacts = map[string]bool{}
 	}
 	if !*noBlock {
-		// The fast path must honor every disable source so a disabled
-		// facterversion query falls through to normal resolution (and
-		// disable-beats-query), mirroring the engine's union.
-		fastPathEntries := append([]string(nil), configOptions.Disabled...)
-		fastPathEntries = append(fastPathEntries, disableEntries...)
-		fastPathEntries = append(fastPathEntries, engine.EnvironmentDisabledFacts(os.Environ())...)
-		disabledFactsForFastPath = engine.DisabledFactsForFiltering(fastPathEntries, configOptions.FactGroups)
+		// The fast path honors every disable source so a disabled facterversion
+		// query falls through to normal resolution (and disable-beats-query). It
+		// derives its set from the same engine union discovery planning uses, so
+		// the two can never disagree on whether facterversion is disabled.
+		disabledFactsForFastPath = engine.DisabledUnion(configOptions, disableEntries, os.Environ())
 	}
 	mergeDottedFacts := configOptions.ForceDotResolution || *forceDotResolution
 	logLevel := firstNonEmpty(flags.Lookup("log-level").Value.String(), flags.Lookup("l").Value.String(), configOptions.LogLevel)
@@ -496,19 +494,14 @@ func canUseVersionQueryFastPath(queries, externalDirs []string, disabledFacts ma
 
 func writeVersionQuery(stdout io.Writer, jsonOutput, yamlOutput, hoconOutput bool) error {
 	facts := []engine.ResolvedFact{{Name: "facterversion", Value: engine.Version, UserQuery: "facterversion"}}
-	var (
-		out string
-		err error
-	)
-	if jsonOutput {
-		out, err = engine.FormatJSON(facts)
-	} else if yamlOutput {
-		out = engine.FormatYAML(facts)
-	} else if hoconOutput {
-		out = engine.FormatHOCON(facts)
-	} else {
-		out = engine.FormatLegacy(facts)
-	}
+	// The fast path carries only the three format booleans: it deliberately
+	// ignores --color and --force-dot-resolution, so Colorize/IncludeTypedDotted
+	// stay false and formatter selection reuses the engine's own precedence.
+	out, err := engine.BuildFormatter(engine.FormatOptions{
+		JSON:  jsonOutput,
+		YAML:  yamlOutput,
+		HOCON: hoconOutput,
+	}).Format(facts)
 	if err != nil {
 		return err
 	}

internal/app/app_test.go

@@ -58,6 +58,38 @@ func TestRun_shortVersion(t *testing.T) {
 	}
 }
 
+// Byte pins for the version fast path's formatter selection. These lock the
+// exact stdout before writeVersionQuery routes through engine.BuildFormatter,
+// so the reroute is proven byte-identical.
+func TestRun_facterversionJSONFastPathBytes(t *testing.T) {
+	var stdout, stderr bytes.Buffer
+
+	if err := Run(&stdout, &stderr, []string{"--json", "facterversion"}); err != nil {
+		t.Fatal(err)
+	}
+	want := "{\n  \"facterversion\": \"" + engine.Version + "\"\n}\n"
+	if got := stdout.String(); got != want {
+		t.Fatalf("stdout = %q, want %q", got, want)
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+func TestRun_facterversionHOCONFastPathBytes(t *testing.T) {
+	var stdout, stderr bytes.Buffer
+
+	if err := Run(&stdout, &stderr, []string{"--hocon", "facterversion"}); err != nil {
+		t.Fatal(err)
+	}
+	if got, want := stdout.String(), engine.Version+"\n"; got != want {
+		t.Fatalf("stdout = %q, want %q", got, want)
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("stderr = %q, want empty", stderr.String())
+	}
+}
+
 func TestRun_facterversionQueryAllowsExternalOverride(t *testing.T) {
 	dir := t.TempDir()
 	if err := os.WriteFile(filepath.Join(dir, "version.txt"), []byte("facterversion=external\n"), 0o600); err != nil {

internal/app/disable_test.go

@@ -76,3 +76,37 @@ func TestRun_noBlockClearsDisableOption(t *testing.T) {
 		t.Fatalf("stdout = %q, want --no-block to clear --disable and resolve alpha", stdout.String())
 	}
 }
+
+// The version fast path must fall through to full discovery when facterversion
+// is disabled by an ambient source, so a disabled version query behaves like
+// any other disabled fact. Pinned in the bare legacy format (the only format
+// whose disabled-single-query stdout is literally empty) before the fast path
+// consumes the engine disabled-union, so the reroute cannot alter fall-through.
+func TestRun_facterversionDisabledByEnvFallsThrough(t *testing.T) {
+	t.Setenv("FACTS_DISABLE", "facterversion")
+	var stdout, stderr bytes.Buffer
+
+	if err := Run(&stdout, &stderr, []string{"facterversion"}); err != nil {
+		t.Fatal(err)
+	}
+	if stdout.Len() != 0 {
+		t.Fatalf("stdout = %q, want empty (disabled facterversion falls through)", stdout.String())
+	}
+	if got, want := stderr.String(), "WARN Facts - fact \"facterversion\" is disabled by FACTS_DISABLE\n"; got != want {
+		t.Fatalf("stderr = %q, want %q", got, want)
+	}
+}
+
+func TestRun_facterversionDisabledByFlagFallsThrough(t *testing.T) {
+	var stdout, stderr bytes.Buffer
+
+	if err := Run(&stdout, &stderr, []string{"--disable", "facterversion", "facterversion"}); err != nil {
+		t.Fatal(err)
+	}
+	if stdout.Len() != 0 {
+		t.Fatalf("stdout = %q, want empty (disabled facterversion falls through)", stdout.String())
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("stderr = %q, want no diagnostic for --disable", stderr.String())
+	}
+}

internal/engine/augeas.go

@@ -5,15 +5,15 @@ import "regexp"
 var augeasVersionPattern = regexp.MustCompile(`\b(\d+\.\d+(?:\.\d+)?)\b`)
 
 func probeAugeasVersion(s *Session) string {
-	return currentAugeasVersion(fileExists, s.commandOutput)
+	return currentAugeasVersion(s)
 }
 
-func currentAugeasVersion(exists func(string) bool, run commandRunner) string {
+func currentAugeasVersion(s *Session) string {
 	augparse := "augparse"
-	if exists("/opt/puppetlabs/puppet/bin/augparse") {
+	if fileExists(s.host, "/opt/puppetlabs/puppet/bin/augparse") {
 		augparse = "/opt/puppetlabs/puppet/bin/augparse"
 	}
-	return parseAugeasVersion(run(augparse, "--version"))
+	return parseAugeasVersion(s.commandOutput(augparse, "--version"))
 }
 
 func parseAugeasVersion(out string) string {

internal/engine/augeas_test.go

@@ -1,6 +1,7 @@
 package engine
 
 import (
+	"os"
 	"reflect"
 	"testing"
 )
@@ -34,45 +35,45 @@ func TestAugeasVersionFacts_omittedWhenAugparseUnavailable(t *testing.T) {
 }
 
 func TestCurrentAugeasVersion_prefersPuppetAgentAugparse(t *testing.T) {
-	var gotName string
-	var gotArgs []string
-
-	got := currentAugeasVersion(
-		func(path string) bool { return path == "/opt/puppetlabs/puppet/bin/augparse" },
-		func(name string, args ...string) string {
-			gotName = name
-			gotArgs = args
-			return "augparse 1.12.0 <http://augeas.net/>"
+	host := &fakeHostOS{
+		emptyRunDefault: true,
+		stats:           map[string]os.FileInfo{"/opt/puppetlabs/puppet/bin/augparse": fakeFileInfo{name: "augparse"}},
+		runOutputs: map[string]string{
+			fakeRunKey("/opt/puppetlabs/puppet/bin/augparse", "--version"): "augparse 1.12.0 <http://augeas.net/>",
 		},
-	)
+	}
+	s := NewSession()
+	s.host = host
+
+	got := currentAugeasVersion(s)
 
 	if got != "1.12.0" {
 		t.Fatalf("currentAugeasVersion() = %q, want 1.12.0", got)
 	}
-	if gotName != "/opt/puppetlabs/puppet/bin/augparse" {
-		t.Fatalf("augparse command = %q, want puppet-agent augparse", gotName)
-	}
-	if !reflect.DeepEqual(gotArgs, []string{"--version"}) {
-		t.Fatalf("augparse args = %#v, want --version", gotArgs)
+	want := []fakeHostRunCall{{name: "/opt/puppetlabs/puppet/bin/augparse", args: []string{"--version"}}}
+	if !reflect.DeepEqual(host.runCalls, want) {
+		t.Fatalf("commands = %#v, want puppet-agent augparse --version", host.runCalls)
 	}
 }
 
 func TestCurrentAugeasVersion_usesPathAugparseWhenPuppetAgentAugparseIsAbsent(t *testing.T) {
-	var gotName string
-
-	got := currentAugeasVersion(
-		func(string) bool { return false },
-		func(name string, args ...string) string {
-			gotName = name
-			return "augparse 1.14.1 <http://augeas.net/>"
+	host := &fakeHostOS{
+		emptyRunDefault: true,
+		runOutputs: map[string]string{
+			fakeRunKey("augparse", "--version"): "augparse 1.14.1 <http://augeas.net/>",
 		},
-	)
+	}
+	s := NewSession()
+	s.host = host
+
+	got := currentAugeasVersion(s)
 
 	if got != "1.14.1" {
 		t.Fatalf("currentAugeasVersion() = %q, want 1.14.1", got)
 	}
-	if gotName != "augparse" {
-		t.Fatalf("augparse command = %q, want path augparse", gotName)
+	want := []fakeHostRunCall{{name: "augparse", args: []string{"--version"}}}
+	if !reflect.DeepEqual(host.runCalls, want) {
+		t.Fatalf("commands = %#v, want path augparse --version", host.runCalls)
 	}
 }
 

internal/engine/az.go

@@ -3,7 +3,6 @@ package engine
 import (
 	"context"
 	"encoding/json"
-	"io"
 	"net/http"
 	"strings"
 	"time"
@@ -13,7 +12,6 @@ const (
 	azureMetadataBaseURL = "http://169.254.169.254"
 	azureAPIVersion      = "2020-09-01"
 	azureRequestTimeout  = 5 * time.Second
-	azureMaxBodyBytes    = 1 << 20
 )
 
 type azureClient struct {
@@ -23,12 +21,7 @@ type azureClient struct {
 
 func newAzureClient(baseURL string, httpClient *http.Client) *azureClient {
 	if httpClient == nil {
-		httpClient = &http.Client{
-			Timeout: azureRequestTimeout,
-			Transport: &http.Transport{
-				Proxy: nil,
-			},
-		}
+		httpClient = newMetadataHTTPClient(azureRequestTimeout)
 	}
 	return &azureClient{baseURL: strings.TrimRight(baseURL, "/"), httpClient: httpClient}
 }
@@ -52,25 +45,14 @@ func azureHypervisor(name string) bool {
 }
 
 func (ac *azureClient) metadata(ctx context.Context) map[string]any {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, ac.baseURL+"/metadata/instance?api-version="+azureAPIVersion, nil)
-	if err != nil {
-		return map[string]any{}
-	}
-	req.Header.Set("Metadata", "true")
-	resp, err := ac.httpClient.Do(req)
-	if err != nil {
-		return map[string]any{}
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return map[string]any{}
-	}
-	data, err := io.ReadAll(io.LimitReader(resp.Body, azureMaxBodyBytes))
-	if err != nil {
+	body, _, ok := fetchMetadata(ctx, ac.httpClient, http.MethodGet, ac.baseURL+"/metadata/instance?api-version="+azureAPIVersion, map[string]string{
+		"Metadata": "true",
+	})
+	if !ok {
 		return map[string]any{}
 	}
 	metadata := map[string]any{}
-	if err := json.Unmarshal(data, &metadata); err != nil {
+	if err := json.Unmarshal([]byte(body), &metadata); err != nil {
 		return map[string]any{}
 	}
 	return metadata

internal/engine/config_test.go

@@ -1009,7 +1009,7 @@ func TestGroupTTLSecondsRejectsMalformedTTLTokens(t *testing.T) {
 }
 
 func TestDisabledFactsForFiltering_retiredLegacyGroupBlocksNothing(t *testing.T) {
-	blocked := DisabledFactsForFiltering([]string{"legacy"}, nil)
+	blocked := DisabledFactsWithGroups([]string{"legacy"}, nil)
 
 	facts := []ResolvedFact{
 		{Name: "os.name", Value: "Darwin"},
@@ -1327,3 +1327,37 @@ func TestFirstConfigValueReturnsFirstNonEmptyValue(t *testing.T) {
 		t.Fatalf("firstConfigValue(all empty) = %q, want empty", got)
 	}
 }
+
+func TestDisabledUnion_mergesConfigFlagAndEnvSources(t *testing.T) {
+	config := Config{Disabled: []string{"os"}}
+	got := DisabledUnion(config, []string{"processors"}, []string{"FACTS_DISABLE=networking"})
+	for _, name := range []string{"os", "processors", "networking"} {
+		if !got[name] {
+			t.Fatalf("DisabledUnion() missing %q from the union; got %#v", name, got)
+		}
+	}
+}
+
+func TestDisabledUnion_nilEnvironExcludesEnvSource(t *testing.T) {
+	config := Config{Disabled: []string{"os"}}
+	got := DisabledUnion(config, nil, nil)
+	if !got["os"] {
+		t.Fatalf("DisabledUnion() dropped config.Disabled; got %#v", got)
+	}
+	if got["networking"] {
+		t.Fatalf("DisabledUnion(nil environ) included an env source; got %#v", got)
+	}
+}
+
+func TestDisabledUnion_expandsGroups(t *testing.T) {
+	config := Config{
+		Disabled:   []string{"web"},
+		FactGroups: []FactGroup{{Name: "web", Facts: []string{"networking", "os.name"}}},
+	}
+	got := DisabledUnion(config, nil, nil)
+	for _, name := range []string{"networking", "os.name"} {
+		if !got[name] {
+			t.Fatalf("DisabledUnion() did not expand group member %q; got %#v", name, got)
+		}
+	}
+}