Fix bug with path arg type and add env var expansion (#571)

Summary:
Pull Request resolved: #571

The path arg type was not being evaluated based on the path of the TTP YAML. This caused relative paths such as `default: <FILE>` or `default: ./<FILE>` to resolve based on the path you executed ttpforge from as opposed to the path of the YAML.

This has been updated to resolve relative to the YAML when provided as a default value and resolve relative to the current working directory when provided as a command-line argument.

Additionally added handling for variable expansion within the shell. This should be helpful for TTPs referencing variables like $HOME or alike.

Reviewed By: RoboticPrism

Differential Revision: D85156301

fbshipit-source-id: 69d8db00c37576684a03ef30d0d55ba078ec2e61

Author: isaac-fletcher

docs/foundations/args.md

@@ -55,20 +55,27 @@ TTPForge supports the following argument types (which you can specify with the
 
 ## The `path` Argument Type
 
-If your TTP will accept arguments that refer to file paths on disk, you should
-**almost always** use `type: path` when declaring those arguments, as shown in
-the example below (just focus on the `args:` section for now, though you can
-check out the [edit_file documentation](actions/edit_file.md) if you are
-curious):
-
-https://github.com/facebookincubator/TTPForge/blob/7634dc65879ec43a108a4b2d44d7eb2105a2a4b1/example-ttps/actions/edit-file/append-delete.yaml#L1-L35
-
-You must use `type: path` because when you execute `ttpforge run [ttp]`,
-**TTPForge changes its working directory to the folder containing the TTP.**
-This means that relative paths such as `foo/bar` won't retain their original
-meaning by default - however, when you declare your argument using `type: path`,
-TTPForge knows to expand its value to an absolute path prior to changing
-directories, ensuring that everything will work as intended.
+Use `type: path` for file path arguments. Path arguments are automatically:
+
+- **Resolved to absolute paths** with symlinks resolved
+- **Expanded for variables** like `$HOME`, `${USER}`, `~`, etc.
+
+Relative paths resolve differently depending on where they're defined:
+
+- **Default values** (in YAML): Resolved relative to the TTP's directory
+- **CLI arguments**: Resolved relative to where you run `ttpforge`
+
+**Example:**
+
+```yaml
+args:
+  - name: config_file
+    type: path
+    default: ./config.yaml        # Relative to TTP directory
+  - name: output_dir
+    type: path
+    default: $HOME/output         # Variable expansion
+```
 
 ## Predefined Choices for Argument Values
 

example-ttps/args/path.yaml

@@ -0,0 +1,46 @@
+---
+api_version: 2.0
+uuid: 8f3c9d2a-1b5e-4c7a-9f8e-2d3a4b5c6d7e
+name: Path Argument Demo
+description: |
+  This TTP demonstrates how path arguments with default values work.
+  The default path is relative to the YAML file's directory, NOT the
+  directory where ttpforge was executed.
+
+  Example Usage:
+    # Create a test file in the same directory as this YAML
+    echo "Hello from YAML directory" > example-ttps/args/test-file.txt
+
+    # Run with default (will read the file relative to YAML location)
+    ttpforge run example-ttps/args/path-default-relative.yaml
+
+    # Or override with your own file (relative to where you run ttpforge)
+    echo "Hello from execution directory" > my-file.txt
+    ttpforge run example-ttps/args/path-default-relative.yaml --arg file_path=my-file.txt
+
+args:
+  - name: new_dir
+    type: path
+    description: Path to change directory to
+    default: $HOME
+  - name: file_path
+    type: path
+    description: Path to a file to read
+    default: ./test-file.txt
+
+steps:
+  - name: show_current_directory
+    description: |
+      Show the current directory
+    inline: |
+      echo "Current Directory: $(pwd)"
+  - name: change_directory
+    description: |
+      Change directory to demonstrate how path arguments are resolved
+    cd: {{ .Args.new_dir }}
+  - name: print_file_contents
+    description: |
+      Reads the contents of the file provided as an argument
+    inline: |
+      echo Reading file: {{ .Args.file_path }}
+      cat {{.Args.file_path}}

example-ttps/args/test-file.txt

@@ -0,0 +1,3 @@
+Hello from the YAML directory!
+This file is in the same directory as path-default-relative.yaml.
+When you run the TTP with the default argument, this file will be read.

pkg/args/spec.go

@@ -22,6 +22,7 @@ package args
 import (
 	"errors"
 	"fmt"
+	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -47,13 +48,14 @@ type Spec struct {
 //
 // specs: slice of argument Spec values loaded from the TTP yaml
 // argKvStrs: slice of arguments in "ARG_NAME=ARG_VALUE" format
+// cliBaseDir: the directory to resolve CLI path arguments relative to
+// defaultBaseDir: the directory to resolve default path values relative to
 //
 // **Returns:**
 //
-// map[string]string: the parsed and validated argument key-value pairs
+// map[string]any: the parsed and validated argument key-value pairs
 // error: an error if there is a problem
-func ParseAndValidate(specs []Spec, argsKvStrs []string) (map[string]any, error) {
-
+func ParseAndValidate(specs []Spec, argsKvStrs []string, cliBaseDir string, defaultBaseDir string) (map[string]any, error) {
 	// validate the specs
 	processedArgs := make(map[string]any)
 	specsByName := make(map[string]Spec)
@@ -68,12 +70,20 @@ func ParseAndValidate(specs []Spec, argsKvStrs []string) (map[string]any, error)
 		}
 
 		// set the default value, will be overwritten by passed value
+		// Path defaults are resolved relative to defaultBaseDir (typically the YAML directory)
 		if spec.Default != "" {
 			if !spec.isValidChoice(spec.Default) {
 				return nil, fmt.Errorf("invalid default value: %v, allowed values: %v ", spec.Default, strings.Join(spec.Choices, ", "))
 			}
 
-			defaultVal, err := spec.convertArgToType(spec.Default)
+			// For path types, resolve relative paths relative to defaultBaseDir
+			// Absolute paths and paths with shell variables are left as-is
+			defaultValue := spec.Default
+			if spec.Type == "path" && !filepath.IsAbs(spec.Default) && !fileutils.ContainsShellVariable(spec.Default) {
+				defaultValue = filepath.Join(defaultBaseDir, spec.Default)
+			}
+
+			defaultVal, err := spec.convertArgToType(defaultValue)
 			if err != nil {
 				return nil, fmt.Errorf("default value type does not match spec: %w", err)
 			}
@@ -123,7 +133,14 @@ func ParseAndValidate(specs []Spec, argsKvStrs []string) (map[string]any, error)
 			return nil, fmt.Errorf("invalid value format: %v, expected regex format: %v ", argVal, spec.Format)
 		}
 
-		typedVal, err := spec.convertArgToType(argVal)
+		// For path types, resolve relative paths relative to cliBaseDir
+		// Absolute paths and paths with shell variables are left as-is
+		argValue := argVal
+		if spec.Type == "path" && !filepath.IsAbs(argVal) && !fileutils.ContainsShellVariable(argVal) {
+			argValue = filepath.Join(cliBaseDir, argVal)
+		}
+
+		typedVal, err := spec.convertArgToType(argValue)
 		if err != nil {
 			return nil, fmt.Errorf(
 				"failed to process value '%v' specified for argument '%v': %v",
@@ -164,6 +181,8 @@ func (spec Spec) convertArgToType(val string) (any, error) {
 		}
 		return asBool, nil
 	case "path":
+		// Path has already been resolved relative to appropriate baseDir
+		// Just convert to absolute and resolve symlinks
 		absPath, err := fileutils.AbsPath(val)
 		if err != nil {
 			return nil, fmt.Errorf("failed to process argument of type `path`: %w", err)