From 1c79ec3aa1a0395ea150e4e469a778b6f60a9750 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 20 Oct 2026 00:20:38 +0000 Subject: [PATCH] Preserve native import alias identity --- docs/roadmap/remediation.md | 11 +++- facade_import_alias_test.go | 99 +++++++++++++++++++++++++++++ internal/engine/glob.go | 5 ++ internal/engine/import_selection.go | 10 ++- 4 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 facade_import_alias_test.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index b18cfa2..698c863 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Import Discovery and Scheduling Review Findings -Status: open; six independently reproduced findings require remediation. +Status: in progress; issue 1 complete, five findings remain. Reviewed implementation: `aa93aed..15943e2`, ending at `15943e233ca3cd1286c03c0e7085871db344cd4b`, against the accepted @@ -14,7 +14,7 @@ and discovery edge cases, not a proposal to change the accepted architecture. Implement fixes with focused regressions, preserving the selected semantics and existing resource limits. This review made no implementation changes. -## 1. [ ] P1 — Preserve native source identity and relative import bases +## 1. [x] P1 — Preserve native source identity and relative import bases Owners: [directory selection](../../internal/engine/import_selection.go), especially assignment of `target.identity` to a file candidate's `accessPath` @@ -50,6 +50,13 @@ to native filesystems. Verify path, directory, and glob equivalence for a file alias and an alias directory, including nested imports, source context, origins, and existing repeated-source behavior. Keep host canonicalization unchanged. +Completed: native directory and glob discovery retain namespace admission paths, +while traversal cycle identities remain resolved. Added public regressions for +file and directory aliases across all three selectors, helper/resource origins, +source contexts, repeated paths, and distinct alias admission. The regression +failed before the fix. Focused discovery/resource tests and `./scripts/check.sh` +passed with Go 1.26.9 and offline dependency selection. + ## 2. [ ] P1 — Scope main-version protection to the actual effective path Owner: [composePendingItems](../../internal/engine/imports.go), around lines diff --git a/facade_import_alias_test.go b/facade_import_alias_test.go new file mode 100644 index 0000000..8e05aae --- /dev/null +++ b/facade_import_alias_test.go @@ -0,0 +1,99 @@ +package configkit_test + +import ( + "errors" + "io/fs" + "os" + "path" + "path/filepath" + "testing" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +func TestFacadeNativeDiscoveryPreservesAliasIdentity(t *testing.T) { + root := t.TempDir() + for name, contents := range map[string]string{ + "data/item.yml": "configkit:\n imports:\n - path: helper.yml\nvalue: item\n", + "data/helper.yml": "owner: target\nresource: resource.txt\n", + "aliases/helper.yml": "owner: alias\nresource: resource.txt\n", + "data/resource.txt": "target resource", + "aliases/resource.txt": "alias resource", + } { + filename := filepath.Join(root, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(filename), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filename, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + } + for name, target := range map[string]string{"aliases/item.yml": "../data/item.yml", "linked": "data"} { + if err := os.Symlink(target, filepath.Join(root, filepath.FromSlash(name))); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + } + filesystem := configkit.NewFilesystem("aliases", os.DirFS(root)) + type configuration struct { + Value string `yaml:"value"` + Owner string `yaml:"owner"` + Resource string `yaml:"resource"` + } + for _, test := range []struct { + name, declaration, item, helper, owner, resource string + }{ + {"file/path", "path: aliases/item.yml", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"}, + {"file/dir", "dir: aliases\n pattern: item.yml", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"}, + {"file/glob", "glob: 'aliases/ite*.yml'", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"}, + {"directory/path", "path: linked/item.yml", "linked/item.yml", "linked/helper.yml", "target", "target resource"}, + {"directory/dir", "dir: linked\n pattern: item.yml", "linked/item.yml", "linked/helper.yml", "target", "target resource"}, + {"directory/glob", "glob: 'lin*/ite*.yml'", "linked/item.yml", "linked/helper.yml", "target", "target resource"}, + } { + t.Run(test.name, func(t *testing.T) { + result, err := configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+test.declaration+"\n")), configkit.Options[configuration]{ + Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."}, + }) + if err != nil { + t.Fatal(err) + } + if result.Config.Owner != test.owner || result.Config.Value != "item" { + t.Fatalf("configuration = %#v", result.Config) + } + sources := result.Sources() + if len(sources) != 3 || sources[1].Path != test.item || sources[2].Path != test.helper { + t.Fatalf("source identities = %#v", sources) + } + for field, source := range map[string]configkit.SourceInfo{"value": sources[1], "resource": sources[2]} { + info, ok := result.Metadata().Lookup(configkit.Keys(field)) + if !ok || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != source.ID { + t.Fatalf("%s origin = %#v", field, info) + } + context, ok := result.SourceContext(source.ID) + if !ok || context.BaseDir != path.Dir(source.Path) { + t.Fatalf("%s context = %#v", field, context) + } + contents, err := fs.ReadFile(context.FS, path.Join(context.BaseDir, result.Config.Resource)) + if err != nil || string(contents) != test.resource { + t.Fatalf("%s resource = %q, %v", field, contents, err) + } + } + }) + } + // The same native path is still a repeat when separately included. An alias + // to a leaf file remains a different native namespace path, as for path imports. + for _, selector := range []string{"path: aliases/item.yml", "dir: aliases\n pattern: item.yml", "glob: 'aliases/ite*.yml'"} { + _, err := configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+selector+"\n - path: aliases/item.yml\n")), configkit.Options[configuration]{ + Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."}, + }) + var loadError *configkit.LoadError + if !errors.As(err, &loadError) || len(loadError.Diagnostics()) != 1 || loadError.Diagnostics()[0].Code != configkit.CodeImportRepeated { + t.Fatalf("repeated source: %v", err) + } + _, err = configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+selector+"\n - path: data/item.yml\n")), configkit.Options[configuration]{ + Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."}, + }) + if !errors.As(err, &loadError) || len(loadError.Diagnostics()) == 0 || loadError.Diagnostics()[0].Code != configkit.CodeComposeConflict { + t.Fatalf("distinct native alias paths should reach composition: %v", err) + } + } +} diff --git a/internal/engine/glob.go b/internal/engine/glob.go index 37bddc4..1f4e612 100644 --- a/internal/engine/glob.go +++ b/internal/engine/glob.go @@ -79,6 +79,11 @@ func selectGlobImportCandidates(access *filesystemAccess, basePath string, spec continue } physicalPath := target.identity + // Native source identity and relative imports use the namespace + // spelling; resolved link identities belong only to discovery. + if access.native != nil { + physicalPath = accessPath + } if physicalPath == "" { physicalPath, err = access.canonical(accessPath) if err != nil { diff --git a/internal/engine/import_selection.go b/internal/engine/import_selection.go index b3fdb9d..56a940d 100644 --- a/internal/engine/import_selection.go +++ b/internal/engine/import_selection.go @@ -115,7 +115,9 @@ func selectDirectoryImportCandidates(access *filesystemAccess, basePath string, if targetErr != nil { return importSelection{}, []Diagnostic{*importDirectoryTargetDiagnostic(root.site, targetErr)} } - root.accessPath = rootTarget.identity + if access.native == nil { + root.accessPath = rootTarget.identity + } entries, missing, diagnostic := importDirectoryEntries(access, root.accessPath, root.site, limits, accounting) if diagnostic != nil { return importSelection{}, []Diagnostic{*diagnostic} @@ -175,13 +177,15 @@ func selectDirectoryImportCandidates(access *filesystemAccess, basePath string, return importSelection{}, []Diagnostic{*childDiagnostic} } if !vanished { - candidate.accessPath = target.identity + if access.native == nil { + candidate.accessPath = target.identity + } activeDirectories[target.identity] = struct{}{} stack = append(stack, importDirectoryFrame{directory: candidate, depth: frame.depth + 1, entries: childEntries, identity: target.identity}) } continue } - if target.identity != "" { + if target.identity != "" && access.native == nil { candidate.accessPath = target.identity } else { canonical, canonicalErr := access.canonical(accessPath)