Track active symlink expansions precisely

docs/roadmap/remediation.md

@@ -1,6 +1,6 @@
 # Import Discovery and Scheduling Review Findings
 
-Status: in progress; issues 1–3 complete, three findings remain.
+Status: in progress; issues 1–4 complete, two findings remain.
 
 Reviewed implementation: `aa93aed..15943e2`, ending at
 `15943e233ca3cd1286c03c0e7085871db344cd4b`, against the accepted
@@ -120,7 +120,7 @@ undergo their existing containment checks. Public directory/glob regressions
 cover ordinary and aliased roots plus escaping links. All four valid cases
 failed before the fix. The full everyday check passed.
 
-## 4. [ ] P2 — Distinguish finite symlink reuse from actual cycles
+## 4. [x] P2 — Distinguish finite symlink reuse from actual cycles
 
 Owners: [resolveHostImportPath and resolveNativeImportPath](../../internal/engine/filesystem_links.go),
 especially the `activeLinks` sets around lines 115–134 and 172–194.
@@ -143,6 +143,13 @@ separate active-path directory-cycle rule. Add finite-reuse and real-loop
 regressions for host and link-aware native access; do not replace this defect
 with an arbitrary new link or path-depth restriction.
 
+Completed: host and native resolvers now release active links when each target
+expansion completes, using explicit frames rather than retaining a whole-path
+visited set. Added resolver regressions for finite reuse, self loops, and mutual
+loops, plus public host/native directory/glob loading coverage. Finite cases
+failed before the change; all pass now alongside existing traversal-cycle and
+containment tests. The full everyday check passed; no new limit was added.
+
 ## 5. [ ] P2 — Validate the complete depth token before saturating its value
 
 Owner: [parseImportDepth](../../internal/engine/import_spec.go), around lines

facade_import_root_test.go

@@ -50,3 +50,32 @@ func TestFacadeNativeDiscoveryWithNestedRoot(t *testing.T) {
 		}
 	}
 }
+
+func TestFacadeDiscoveryAllowsFiniteLinkReuse(t *testing.T) {
+	root := t.TempDir()
+	if err := os.Mkdir(filepath.Join(root, "parts"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "parts/value.yml"), []byte("value: 1\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("..", filepath.Join(root, "parts/up")); err != nil {
+		t.Skipf("symlinks unavailable: %v", err)
+	}
+	filesystem := configkit.NewFilesystem("finite", os.DirFS(root))
+	for _, native := range []bool{false, true} {
+		for _, selector := range []string{"dir: parts/up/parts/up/parts\n      pattern: '*.yml'", "glob: 'parts/up/parts/up/parts/*.yml'"} {
+			context := &configkit.FilesystemContext{Root: root, BaseDir: root}
+			if native {
+				context = &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."}
+			}
+			result, err := configkit.Load(configkit.Bytes("main", []byte("configkit:\n  imports:\n    - "+selector+"\n")), configkit.Options[map[string]any]{Imports: true, Filesystem: context})
+			if err != nil {
+				t.Fatalf("native=%v selector=%s: %v", native, selector, err)
+			}
+			if result.Config["value"] != int64(1) {
+				t.Fatalf("configuration = %#v", result.Config)
+			}
+		}
+	}
+}

internal/engine/filesystem_links.go

@@ -109,13 +109,26 @@ func (a *filesystemAccess) importEntryTarget(name string, entry fs.DirEntry, fol
 	return importEntryTarget{symlink: symlink, dir: info.IsDir(), identity: name}, nil
 }
 
+// A link stays active until its target has resolved, not until the entire
+// original pathname has resolved. Remaining segments may reuse a finished link.
+type importLinkFrame struct {
+	pending []string
+	link    string
+}
+
 func resolveHostImportPath(access *filesystemAccess, name string) (string, error) {
-	pending := strings.Split(filepath.ToSlash(name), "/")
+	frames := []importLinkFrame{{pending: strings.Split(filepath.ToSlash(name), "/")}}
 	resolved := ""
 	activeLinks := make(map[string]struct{})
-	for len(pending) > 0 {
-		segment := pending[0]
-		pending = pending[1:]
+	for len(frames) > 0 {
+		frame := &frames[len(frames)-1]
+		if len(frame.pending) == 0 {
+			delete(activeLinks, frame.link)
+			frames = frames[:len(frames)-1]
+			continue
+		}
+		segment := frame.pending[0]
+		frame.pending = frame.pending[1:]
 		candidate := path.Join(resolved, segment)
 		if candidate == "" {
 			candidate = "."
@@ -149,8 +162,7 @@ func resolveHostImportPath(access *filesystemAccess, name string) (string, error
 		if !withinPortableRoot(targetPath) {
 			return "", errImportPathOutsideRoot
 		}
-		remaining := append([]string(nil), pending...)
-		pending = append(strings.Split(targetPath, "/"), remaining...)
+		frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate})
 		resolved = ""
 	}
 	if resolved == "" {
@@ -167,12 +179,18 @@ func resolveNativeImportPath(access *filesystemAccess, filesystem fs.ReadLinkFS,
 	if !fs.ValidPath(name) || !access.contains(name) {
 		return "", fs.ErrInvalid
 	}
-	pending := strings.Split(name, "/")
+	frames := []importLinkFrame{{pending: strings.Split(name, "/")}}
 	resolved := ""
 	activeLinks := make(map[string]struct{})
-	for len(pending) > 0 {
-		segment := pending[0]
-		pending = pending[1:]
+	for len(frames) > 0 {
+		frame := &frames[len(frames)-1]
+		if len(frame.pending) == 0 {
+			delete(activeLinks, frame.link)
+			frames = frames[:len(frames)-1]
+			continue
+		}
+		segment := frame.pending[0]
+		frame.pending = frame.pending[1:]
 		candidate := path.Join(resolved, segment)
 		if candidate == "" {
 			candidate = "."
@@ -205,8 +223,7 @@ func resolveNativeImportPath(access *filesystemAccess, filesystem fs.ReadLinkFS,
 		if !fs.ValidPath(targetPath) || !access.contains(targetPath) {
 			return "", errImportPathOutsideRoot
 		}
-		remaining := append([]string(nil), pending...)
-		pending = append(strings.Split(targetPath, "/"), remaining...)
+		frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate})
 		resolved = ""
 	}
 	if resolved == "" {

internal/engine/filesystem_links_test.go

@@ -0,0 +1,47 @@
+package engine
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestImportLinkResolutionTracksActiveExpansions(t *testing.T) {
+	root := t.TempDir()
+	if err := os.Mkdir(filepath.Join(root, "parts"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	for name, target := range map[string]string{"parts/up": "..", "self": "self", "first": "second", "second": "first"} {
+		if err := os.Symlink(target, filepath.Join(root, filepath.FromSlash(name))); err != nil {
+			t.Skipf("symlinks unavailable: %v", err)
+		}
+	}
+	host, err := openHostAccess(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer host.close()
+	filesystem := os.DirFS(root)
+	links, ok := filesystem.(fs.ReadLinkFS)
+	if !ok {
+		t.Fatal("native directory filesystem does not expose link inspection")
+	}
+	native := newNativeAccess(filesystem, ".", &namespaceToken{})
+	for name, resolve := range map[string]func(string) (string, error){
+		"host":   func(name string) (string, error) { return resolveHostImportPath(host, name) },
+		"native": func(name string) (string, error) { return resolveNativeImportPath(native, links, name) },
+	} {
+		t.Run(name, func(t *testing.T) {
+			if got, err := resolve("parts/up/parts/up/parts"); err != nil || got != "parts" {
+				t.Errorf("finite link reuse = %q, %v", got, err)
+			}
+			for _, cycle := range []string{"self", "first"} {
+				if _, err := resolve(cycle); !errors.Is(err, errImportDirectoryLinkCycle) {
+					t.Errorf("%s cycle = %v", cycle, err)
+				}
+			}
+		})
+	}
+}