From 8b45bd637d0bfb8432d410a6590a63c754f060c1 Mon Sep 17 00:00:00 2001 From: Zander Hill Date: Thu, 26 Feb 2026 03:13:12 -0800 Subject: [PATCH] Add symlink resolution for executable discovery filepath.Walk does not follow symlinks, so symlinked scripts were invisible to help listings, exec, and tab completion. Extract collectExecutables from the help command and resolve symlinks via os.Stat before checking executable bits. Broken symlinks are silently skipped. Also adds self-documenting help target to Makefile. EARS: SYMLINK-001 through SYMLINK-006 --- Makefile | 19 ++-- cmd/help.go | 49 ++++++--- cmd/symlink_test.go | 225 +++++++++++++++++++++++++++++++++++++++++ examples/symlinked-foo | 1 + test/tome-cli.test.ts | 23 +++++ 5 files changed, 296 insertions(+), 21 deletions(-) create mode 100644 cmd/symlink_test.go create mode 120000 examples/symlinked-foo diff --git a/Makefile b/Makefile index 522e33c..74d2ad4 100644 --- a/Makefile +++ b/Makefile @@ -1,15 +1,22 @@ BINARY=bin/tome-cli +.DEFAULT_GOAL := help + +.PHONY: help build clean deps deps-chlogs test test-go test-e2e run tag changelog docs release + +help: ## Show this help + @grep -E '^[a-zA-Z0-9_-]+:.*## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' + all: build test -build: $(BINARY) +build: $(BINARY) ## Build the binary GO_FILES = $(shell find . -name '*.go' | grep -v /vendor/) EMBED_FILES = $(shell find . -name '*.tmpl' | grep -v /vendor/) $(BINARY): $(GO_FILES) $(EMBED_FILES) go build -o bin/tome-cli -clean: +clean: ## Remove build artifacts go clean rm -f $(BINARY) @@ -19,18 +26,18 @@ deps-chlogs: deps: deps-chlogs go mod tidy -test: build test-go test-e2e +test: build test-go test-e2e ## Run all tests (build + unit + e2e) test/bin/wrapper.sh: build @ $(BINARY) --executable wrapper.sh --root examples alias --output test/bin/wrapper.sh -test-e2e: test/bin/wrapper.sh build +test-e2e: test/bin/wrapper.sh build ## Run Deno E2E tests @ deno test --allow-env --allow-read --allow-run test/*.ts -test-go: +test-go: ## Run Go unit tests go test ./... -run: build +run: build ## Run the binary (pass ARGS=... for arguments) $(BINARY) $(ARGS) tag: diff --git a/cmd/help.go b/cmd/help.go index 41ecc4e..261c1f2 100644 --- a/cmd/help.go +++ b/cmd/help.go @@ -5,10 +5,12 @@ package cmd import ( "io/fs" + "os" "path" "path/filepath" "github.com/lithammer/dedent" + gitignore "github.com/sabhiram/go-gitignore" "github.com/spf13/cobra" ) @@ -45,21 +47,7 @@ var helpCmd = &cobra.Command{ rootDir := config.RootDir() ignorePatterns := config.IgnorePatterns() if len(args) == 0 { - allExecutables := []string{} - fn := func(path string, info fs.FileInfo, err error) error { - if err != nil { - return err - } - if info.IsDir() { - return nil - } - if isExecutableByOwner(info.Mode()) && !ignorePatterns.MatchesPath(path) { - allExecutables = append(allExecutables, path) - } - return nil - } - // TODO: does not handle symlinks, consider fb symlinkWalk instead - err := filepath.Walk(rootDir, fn) + allExecutables, err := collectExecutables(rootDir, ignorePatterns) if err != nil { return err } @@ -78,6 +66,37 @@ var helpCmd = &cobra.Command{ ValidArgsFunction: ValidArgsFunctionForScripts, } +// collectExecutables walks rootDir and returns paths to all executable files, +// resolving symlinks to check the target's properties. +// SYMLINK-001, SYMLINK-002: symlinked executables are included. +// SYMLINK-003: broken symlinks are skipped without error. +func collectExecutables(rootDir string, ignorePatterns *gitignore.GitIgnore) ([]string, error) { + var allExecutables []string + fn := func(p string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + // SYMLINK-001, SYMLINK-002: resolve symlinks to check target properties + if info.Mode()&os.ModeSymlink != 0 { + resolved, statErr := os.Stat(p) + if statErr != nil { + // SYMLINK-003: skip broken symlinks + return nil + } + info = resolved + } + if info.IsDir() { + return nil + } + if isExecutableByOwner(info.Mode()) && !ignorePatterns.MatchesPath(p) { + allExecutables = append(allExecutables, p) + } + return nil + } + err := filepath.Walk(rootDir, fn) + return allExecutables, err +} + func init() { rootCmd.AddCommand(helpCmd) } diff --git a/cmd/symlink_test.go b/cmd/symlink_test.go new file mode 100644 index 0000000..8e41b80 --- /dev/null +++ b/cmd/symlink_test.go @@ -0,0 +1,225 @@ +package cmd + +import ( + "os" + "path/filepath" + "sort" + "testing" + + gitignore "github.com/sabhiram/go-gitignore" +) + +// SYMLINK-001: WHEN a symlinked executable file exists in the root directory, +// the help command SHALL list it alongside regular executable files. +func TestCollectExecutables_IncludesSymlinkedFiles(t *testing.T) { + tmpDir := t.TempDir() + setupTestConfig(t, tmpDir, "tome-cli") + + // Create a real executable script + realScript := filepath.Join(tmpDir, "real-script") + if err := os.WriteFile(realScript, []byte("#!/bin/bash\n# USAGE: $0 \necho 1\n"), 0755); err != nil { + t.Fatal(err) + } + + // Create a symlink to it + symlink := filepath.Join(tmpDir, "linked-script") + if err := os.Symlink("real-script", symlink); err != nil { + t.Fatal(err) + } + + ignorePatterns := gitignore.CompileIgnoreLines() + executables, err := collectExecutables(tmpDir, ignorePatterns) + if err != nil { + t.Fatalf("collectExecutables() returned error: %v", err) + } + + // Both real and symlinked scripts should appear + if len(executables) != 2 { + t.Fatalf("expected 2 executables, got %d: %v", len(executables), executables) + } + + sort.Strings(executables) + expected := []string{ + filepath.Join(tmpDir, "linked-script"), + filepath.Join(tmpDir, "real-script"), + } + sort.Strings(expected) + + for i, e := range expected { + if executables[i] != e { + t.Errorf("expected executables[%d]=%s, got %s", i, e, executables[i]) + } + } +} + +// SYMLINK-002: WHEN a symlinked executable file exists in a subdirectory, +// the help command SHALL list it alongside regular executable files in that subdirectory. +func TestCollectExecutables_IncludesSymlinksInSubdirs(t *testing.T) { + tmpDir := t.TempDir() + setupTestConfig(t, tmpDir, "tome-cli") + + subDir := filepath.Join(tmpDir, "subdir") + if err := os.Mkdir(subDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a real executable in subdir + realScript := filepath.Join(subDir, "real-script") + if err := os.WriteFile(realScript, []byte("#!/bin/bash\necho 1\n"), 0755); err != nil { + t.Fatal(err) + } + + // Create a symlink in subdir + symlink := filepath.Join(subDir, "linked-script") + if err := os.Symlink("real-script", symlink); err != nil { + t.Fatal(err) + } + + ignorePatterns := gitignore.CompileIgnoreLines() + executables, err := collectExecutables(tmpDir, ignorePatterns) + if err != nil { + t.Fatalf("collectExecutables() returned error: %v", err) + } + + if len(executables) != 2 { + t.Fatalf("expected 2 executables, got %d: %v", len(executables), executables) + } + + sort.Strings(executables) + expected := []string{ + filepath.Join(subDir, "linked-script"), + filepath.Join(subDir, "real-script"), + } + sort.Strings(expected) + + for i, e := range expected { + if executables[i] != e { + t.Errorf("expected executables[%d]=%s, got %s", i, e, executables[i]) + } + } +} + +// SYMLINK-003: WHEN a symlink points to a non-existent target (broken symlink), +// the help command SHALL skip it without error. +func TestCollectExecutables_SkipsBrokenSymlinks(t *testing.T) { + tmpDir := t.TempDir() + setupTestConfig(t, tmpDir, "tome-cli") + + // Create a real executable + realScript := filepath.Join(tmpDir, "real-script") + if err := os.WriteFile(realScript, []byte("#!/bin/bash\necho 1\n"), 0755); err != nil { + t.Fatal(err) + } + + // Create a broken symlink (target does not exist) + brokenLink := filepath.Join(tmpDir, "broken-link") + if err := os.Symlink("nonexistent-target", brokenLink); err != nil { + t.Fatal(err) + } + + ignorePatterns := gitignore.CompileIgnoreLines() + executables, err := collectExecutables(tmpDir, ignorePatterns) + if err != nil { + t.Fatalf("collectExecutables() returned error: %v", err) + } + + // Only the real script should appear, broken symlink skipped + if len(executables) != 1 { + t.Fatalf("expected 1 executable, got %d: %v", len(executables), executables) + } + if executables[0] != realScript { + t.Errorf("expected %s, got %s", realScript, executables[0]) + } +} + +// SYMLINK-005: WHEN a symlinked executable file exists in the root directory, +// the help command with the script name as argument SHALL display its usage and help text. +func TestNewScript_FollowsSymlinks(t *testing.T) { + tmpDir := t.TempDir() + setupTestConfig(t, tmpDir, "tome-cli") + + // Create a real script with USAGE header + realScript := filepath.Join(tmpDir, "real-script") + content := "#!/bin/bash\n# USAGE: $0 \n# This is help text\n\necho 1\n" + if err := os.WriteFile(realScript, []byte(content), 0755); err != nil { + t.Fatal(err) + } + + // Create a symlink + symlink := filepath.Join(tmpDir, "linked-script") + if err := os.Symlink("real-script", symlink); err != nil { + t.Fatal(err) + } + + // NewScript should parse the symlink target and extract usage/help + s := NewScript(symlink, tmpDir) + usage := s.Usage() + if usage == "" { + t.Error("NewScript on symlink returned empty usage, expected parsed usage from target") + } + if usage != " " { + t.Errorf("expected usage ' ', got '%s'", usage) + } + + help := s.Help() + if help == "" { + t.Error("NewScript on symlink returned empty help, expected parsed help from target") + } +} + +// SYMLINK-006: WHEN a symlinked executable file exists in the root directory, +// the completion system SHALL include it in tab completion results. +func TestCompletionIncludesSymlinks(t *testing.T) { + tmpDir := t.TempDir() + setupTestConfig(t, tmpDir, "tome-cli") + + // Create a real executable script + realScript := filepath.Join(tmpDir, "real-script") + if err := os.WriteFile(realScript, []byte("#!/bin/bash\n# USAGE: $0 \necho 1\n"), 0755); err != nil { + t.Fatal(err) + } + + // Create a symlink + symlink := filepath.Join(tmpDir, "linked-script") + if err := os.Symlink("real-script", symlink); err != nil { + t.Fatal(err) + } + + // ReadDir is used by ValidArgsFunctionForScripts; verify symlink appears + entries, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("ReadDir() returned error: %v", err) + } + + var names []string + for _, entry := range entries { + names = append(names, entry.Name()) + } + + foundLinked := false + foundReal := false + for _, name := range names { + if name == "linked-script" { + foundLinked = true + } + if name == "real-script" { + foundReal = true + } + } + + if !foundLinked { + t.Error("linked-script not found in directory listing for completion") + } + if !foundReal { + t.Error("real-script not found in directory listing for completion") + } + + // Verify the symlinked script is detected as executable via os.Stat (which follows symlinks) + info, err := os.Stat(symlink) + if err != nil { + t.Fatalf("os.Stat on symlink failed: %v", err) + } + if !isExecutableByOwner(info.Mode()) { + t.Error("symlinked script not detected as executable via os.Stat") + } +} diff --git a/examples/symlinked-foo b/examples/symlinked-foo new file mode 120000 index 0000000..1910281 --- /dev/null +++ b/examples/symlinked-foo @@ -0,0 +1 @@ +foo \ No newline at end of file diff --git a/test/tome-cli.test.ts b/test/tome-cli.test.ts index dde7b54..c632e91 100644 --- a/test/tome-cli.test.ts +++ b/test/tome-cli.test.ts @@ -38,12 +38,14 @@ for await (let [executable, fn] of [["tome-cli", tome], ["wrapper.sh", wrapper]] await assertSnapshot(t, out); }); + // SYMLINK-001: symlinked-foo SHALL appear in help listing Deno.test(`top level help`, async function (t): Promise { const { lines } = await fn("help"); assertEquals(lines, [ "folder bar: ", "folder test-env-injection: ", "foo: ", + "symlinked-foo: ", "test-hooks:", ]); }); @@ -98,6 +100,27 @@ for await (let [executable, fn] of [["tome-cli", tome], ["wrapper.sh", wrapper]] ]); }); + // SYMLINK-004: exec SHALL execute a symlinked script + Deno.test(`${executable}: exec runs symlinked script`, async function (t): Promise { + const { code, lines } = await fn(`exec symlinked-foo`); + assertEquals(code, 0); + assertEquals(lines[0], "1"); + }); + + // SYMLINK-005: help SHALL display usage for a symlinked script + Deno.test(`${executable}: help shows symlinked script usage`, async function (t): Promise { + const { code, out } = await fn(`help symlinked-foo`); + assertEquals(code, 0); + assertStringIncludes(out, " "); + assertStringIncludes(out, "This is a foo script"); + }); + + // SYMLINK-006: completion SHALL include symlinked scripts + Deno.test(`${executable}: completion includes symlinked scripts`, async function (t): Promise { + const { out } = await fn(`__complete exec sym`); + assertStringIncludes(out, "symlinked-foo"); + }); + Deno.test(`${executable}: injects TOME_ROOT and TOME_EXECUTABLE into environment of script`, async function (t): Promise { const { code, lines, executable } = await fn(`exec folder test-env-injection`); assertEquals(code, 0);