From c0a3f55f1227b4118863259b441e7858d59a854c Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 10 Oct 2026 15:13:00 +0000 Subject: [PATCH] Resolve module paths consistently through symlinks --- docs/roadmap/remediation.md | 12 +++++++-- internal/releaseidentity/identity.go | 8 +++--- preparation_test.go | 26 +++++++++++------- repository_test.go | 40 ++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 16 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 97558de..440e31a 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Remediation: module release implementation review -Status: open; issues 1–11 are complete, and issues 12–15 require remediation. +Status: open; issues 1–12 are complete, and issues 13–15 require remediation. The initial review examined `90559fe`, compared with planning baseline `dbfae9e`. The follow-up repository audit examined `6b0b8ce`, including those corrections. @@ -423,7 +423,15 @@ transport stub, and a local HTTP server. The artifact-read evidence also used ## Issue 12: Resolve module paths consistently across declaration sources (P2) -Status: open. Category: inconsistent release-unit selection. +Status: complete. Category: inconsistent release-unit selection. + +**Completion:** Filesystem selection follows symlinks before processing parent +components, matching committed traversal. Public build, candidate check, and +hosted page delivery now agree on the selected module and tag for `link/..`; +dependency preparation executes in that same module. The new regression cases +failed before the correction and pass afterward. Focused API/preparation tests +and the complete identity/Git suites pass offline, including aliases, separate +declaration sources, sparse selection, cycles, boundaries, and cancellation. **Evidence:** [Filesystem selection](../../internal/releaseidentity/identity.go) cleans the joined module path before following symlinks. Committed selection diff --git a/internal/releaseidentity/identity.go b/internal/releaseidentity/identity.go index c45a206..9318e7e 100644 --- a/internal/releaseidentity/identity.go +++ b/internal/releaseidentity/identity.go @@ -51,11 +51,9 @@ func ResolveSelection(repositoryDir, moduleDir string) (Selection, error) { if err != nil { return Selection{}, fmt.Errorf("resolve repository directory %q: %w", repositoryDir, err) } - moduleAbs, err := filepath.Abs(filepath.Join(repositoryAbs, moduleDir)) - if err != nil { - return Selection{}, fmt.Errorf("resolve module directory %q: %w", moduleDir, err) - } - moduleAbs, err = filepath.EvalSymlinks(moduleAbs) + // Preserve components until symlinks are followed, so link/.. uses the + // linked directory's parent just as committed path traversal does. + moduleAbs, err := filepath.EvalSymlinks(repositoryAbs + string(filepath.Separator) + moduleDir) if err != nil { return Selection{}, fmt.Errorf("resolve module directory %q: %w", moduleDir, err) } diff --git a/preparation_test.go b/preparation_test.go index 7badd23..7cfbcb8 100644 --- a/preparation_test.go +++ b/preparation_test.go @@ -221,6 +221,12 @@ func TestPrepareBuildDependenciesUsesSelectedModuleDirectory(t *testing.T) { if err := os.MkdirAll(moduleDir, 0o700); err != nil { t.Fatal(err) } + if err := os.Mkdir(filepath.Join(moduleDir, "deep"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink("integrations/ckconfig/deep", filepath.Join(repository, "linked")); err != nil { + t.Fatal(err) + } toolDir := t.TempDir() marker := filepath.Join(t.TempDir(), "go-directory") tool := filepath.Join(toolDir, "go") @@ -229,15 +235,17 @@ func TestPrepareBuildDependenciesUsesSelectedModuleDirectory(t *testing.T) { } t.Setenv("PATH", toolDir+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("PREPARE_DIR_MARKER", marker) - if err := releasekit.PrepareBuildDependencies(context.Background(), releasekit.DependencyPreparationRequest{ - RepositoryDir: repository, - ModuleDir: "integrations/ckconfig", - }); err != nil { - t.Fatal(err) - } - got, err := os.ReadFile(marker) - if err != nil || string(got) != moduleDir { - t.Fatalf("go mod download directory = %q, error=%v, want %q", got, err, moduleDir) + for _, selected := range []string{"integrations/ckconfig", "linked/.."} { + if err := releasekit.PrepareBuildDependencies(context.Background(), releasekit.DependencyPreparationRequest{ + RepositoryDir: repository, + ModuleDir: selected, + }); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(marker) + if err != nil || string(got) != moduleDir { + t.Fatalf("go mod download directory for %q = %q, error=%v, want %q", selected, got, err, moduleDir) + } } } diff --git a/repository_test.go b/repository_test.go index a944681..1093376 100644 --- a/repository_test.go +++ b/repository_test.go @@ -10,6 +10,46 @@ import ( releasekit "gitea.maximumdirect.net/eric/ReleaseKit" ) +func TestModuleSelectionFollowsSymlinkBeforeParentAcrossOperations(t *testing.T) { + repository := newTaggedDeliveryConsumer(t) + const version = "v1.2.3" + const modulePath = "example.com/nested" + const note = "# Fixture v1.2.3\n\nNested release.\n" + addNestedReleaseModule(t, repository, "nested", modulePath, version) + writeFile(t, repository, "nested/docs/releases/"+version+".md", note) + writeFile(t, repository, "nested/deep/placeholder", "tracked directory\n") + if err := os.Symlink("nested/deep", filepath.Join(repository, "link")); err != nil { + t.Fatal(err) + } + buildGit(t, repository, "add", ".") + buildGit(t, repository, "commit", "-m", "Add linked nested module") + wantDir := filepath.Join(repository, "nested") + wantTag := "nested/" + version + built, err := releasekit.Build(t.Context(), releasekit.BuildRequest{ + RepositoryDir: repository, ModuleDir: "link/..", Version: version, + }) + if err != nil || built.ModuleDir != wantDir || built.ModulePath != modulePath || built.Tag != wantTag { + t.Fatalf("build = %#v, error = %v", built, err) + } + checked, err := releasekit.Check(t.Context(), releasekit.ReleaseRequest{ + RepositoryDir: repository, ModuleDir: "link/..", Product: "Fixture", Version: version, + }) + if err != nil || checked.ModuleDir != wantDir || checked.ModulePath != modulePath || checked.Tag != built.Tag { + t.Fatalf("check = %#v, error = %v", checked, err) + } + buildGit(t, repository, "tag", wantTag) + server, remote := newFacadeDeliveryServer(t, repository) + defer server.Close() + remote.tag, remote.noteBody = wantTag, note + delivered, err := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{ + RepositoryDir: repository, ModuleDir: "link/..", Product: "Fixture", Version: version, + ExpectedTag: wantTag, Destination: facadeDeliveryDestination(server.URL), + }) + if err != nil || delivered.State != releasekit.DeliveryComplete || delivered.ModuleDir != wantDir || delivered.ModulePath != modulePath || delivered.Tag != built.Tag { + t.Fatalf("delivery = %#v, error = %v", delivered, err) + } +} + func TestSourceOperationsResolveContainingRepository(t *testing.T) { repo := newPublishFixture(t) const module = "apps/widget"