Refine workspace dependency seams

Author: brandonbloom

docs/spec-review.md

@@ -169,10 +169,12 @@ Incremental progress:
 - `Workspace` carries a narrow Git runner interface, with the production implementation isolated in `realGitRunner`. Workspace-owned Git operations route through that boundary, and a focused test verifies runner injection without creating a Git repository.
 - `Workspace` carries a narrow working-tree filesystem interface for live file reads and writes. Materialization uses resolved absolute worktree paths instead of CWD-relative plan paths, with regression coverage for dirty-path materialization while the process CWD is elsewhere.
 - `Workspace` carries a narrow temp-store interface for Git object directories and synthetic index files. Planning and dry-run temp allocations are injectable, isolated under a test-owned directory in regression coverage, and cleaned up after use.
+- Script loading uses an injectable `scriptSource`, so command-line `run` no longer embeds direct script file reads in CLI argument handling.
+- `Workspace` carries a narrow path-resolution interface for process CWD and symlink resolution. Path resolution can be tested with fake symlink targets instead of real filesystem links.
 
-Remaining implementation gap: path resolution and script reading still use process OS APIs directly. The production Git runner shells out to Git, and most tests use real temporary Git repositories. This means:
+Remaining implementation gap: the production Git runner shells out to Git, and most tests use real temporary Git repositories. This means:
 - Tests cannot model scenarios without touching the filesystem.
-- The architecture diagram's separation between planner, snapshot store, and git backend is not reflected in the code.
+- The architecture diagram's separation between planner, snapshot store, and git backend is only partially reflected in the code.
 
 ---
 

internal/etch/cli.go

@@ -5,8 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
-	"path/filepath"
 	"strings"
 
 	cli "github.com/urfave/cli/v3"
@@ -176,30 +174,6 @@ func executeStatements(opts GlobalOptions, stmts []Statement, stdout, stderr io.
 	return exec.Run(ops, stdout, stderr)
 }
 
-func ParseScriptAt(cwd, path string) ([]Statement, error) {
-	var data []byte
-	var err error
-	name := path
-	if path == "-" {
-		name = "<stdin>"
-		data, err = readStdin()
-	} else {
-		readPath := path
-		if cwd != "" && !filepath.IsAbs(readPath) {
-			readPath = filepath.Join(cwd, readPath)
-		}
-		data, err = os.ReadFile(readPath)
-	}
-	if err != nil {
-		return nil, failf("%s", err)
-	}
-	return ParseScriptBytes(name, data)
-}
-
-var readStdin = func() ([]byte, error) {
-	return io.ReadAll(os.Stdin)
-}
-
 func completeShell(_ context.Context, cmd *cli.Command) {
 	args := cmd.Args().Slice()
 	if len(args) > 0 && strings.HasPrefix(args[len(args)-1], "-") {

internal/etch/git.go

@@ -3,7 +3,6 @@ package etch
 import (
 	"bytes"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 )
@@ -18,48 +17,59 @@ type Workspace struct {
 	git       gitRunner
 	worktree  workingTreeFS
 	temp      workspaceTempStore
+	paths     workspacePaths
 }
 
 type workspaceDeps struct {
 	git      gitRunner
 	worktree workingTreeFS
 	temp     workspaceTempStore
+	paths    workspacePaths
+}
+
+func (d workspaceDeps) withDefaults() workspaceDeps {
+	if d.git == nil {
+		d.git = realGitRunner{}
+	}
+	if d.worktree == nil {
+		d.worktree = osWorkingTreeFS{}
+	}
+	if d.temp == nil {
+		d.temp = osWorkspaceTempStore{}
+	}
+	if d.paths == nil {
+		d.paths = osWorkspacePaths{}
+	}
+	return d
 }
 
 // OpenWorkspace opens the process working directory. Prefer OpenWorkspaceAt
 // when the caller already has an explicit directory.
 func OpenWorkspace(untracked bool) (*Workspace, error) {
-	cwd, err := os.Getwd()
+	return openWorkspace(untracked, workspaceDeps{})
+}
+
+func openWorkspace(untracked bool, deps workspaceDeps) (*Workspace, error) {
+	deps = deps.withDefaults()
+	cwd, err := deps.paths.getwd()
 	if err != nil {
 		return nil, err
 	}
-	return OpenWorkspaceAt(cwd, untracked)
+	return openWorkspaceAt(cwd, untracked, deps)
 }
 
 func OpenWorkspaceAt(cwd string, untracked bool) (*Workspace, error) {
-	return openWorkspaceAt(cwd, untracked, realGitRunner{})
-}
-
-func openWorkspaceAt(cwd string, untracked bool, runner gitRunner) (*Workspace, error) {
-	return openWorkspaceAtWithDeps(cwd, untracked, workspaceDeps{git: runner})
+	return openWorkspaceAt(cwd, untracked, workspaceDeps{})
 }
 
-func openWorkspaceAtWithDeps(cwd string, untracked bool, deps workspaceDeps) (*Workspace, error) {
-	if deps.git == nil {
-		deps.git = realGitRunner{}
-	}
-	if deps.worktree == nil {
-		deps.worktree = osWorkingTreeFS{}
-	}
-	if deps.temp == nil {
-		deps.temp = osWorkspaceTempStore{}
-	}
-	abs, err := filepath.Abs(cwd)
+func openWorkspaceAt(cwd string, untracked bool, deps workspaceDeps) (*Workspace, error) {
+	deps = deps.withDefaults()
+	abs, err := deps.paths.abs(cwd)
 	if err != nil {
 		return nil, err
 	}
 	cwd = abs
-	if real, err := filepath.EvalSymlinks(cwd); err == nil {
+	if real, err := deps.paths.evalSymlinks(cwd); err == nil {
 		cwd = real
 	} else {
 		return nil, failf("cannot resolve working directory %s: %v", cwd, err)
@@ -69,7 +79,7 @@ func openWorkspaceAtWithDeps(cwd string, untracked bool, deps workspaceDeps) (*W
 		return nil, failf("not inside a git worktree")
 	}
 	root := strings.TrimSpace(string(rootBytes))
-	if real, err := filepath.EvalSymlinks(root); err == nil {
+	if real, err := deps.paths.evalSymlinks(root); err == nil {
 		root = real
 	}
 	headBytes, err := deps.git.output(cwd, nil, "rev-parse", "--verify", "HEAD")
@@ -85,7 +95,7 @@ func openWorkspaceAtWithDeps(cwd string, untracked bool, deps workspaceDeps) (*W
 	if err == nil {
 		ref = strings.TrimSpace(string(refBytes))
 	}
-	return &Workspace{CWD: cwd, Root: root, Head: head, Ref: ref, Unborn: unborn, Untracked: untracked, git: deps.git, worktree: deps.worktree, temp: deps.temp}, nil
+	return &Workspace{CWD: cwd, Root: root, Head: head, Ref: ref, Unborn: unborn, Untracked: untracked, git: deps.git, worktree: deps.worktree, temp: deps.temp, paths: deps.paths}, nil
 }
 
 type ResolvedPath struct {
@@ -120,14 +130,15 @@ func (w *Workspace) Resolve(input string, mayBeMissing bool, finalSymlink bool)
 	if mayBeMissing {
 		checkAbs = filepath.Dir(abs)
 	}
+	paths := w.pathResolver()
 	if finalSymlink && !mayBeMissing {
-		if real, err := filepath.EvalSymlinks(abs); err == nil {
+		if real, err := paths.evalSymlinks(abs); err == nil {
 			checkAbs = real
 			abs = real
 		} else if !isNoSuch(err) {
 			return ResolvedPath{}, failf("%s: %v", input, err)
 		}
-	} else if real, err := filepath.EvalSymlinks(checkAbs); err == nil {
+	} else if real, err := paths.evalSymlinks(checkAbs); err == nil {
 		checkAbs = real
 	} else if !isNoSuch(err) {
 		return ResolvedPath{}, failf("%s: %v", input, err)

internal/etch/git_object_store.go

@@ -57,7 +57,7 @@ func (w *Workspace) objectDir() (string, error) {
 	if !filepath.IsAbs(path) {
 		path = filepath.Join(w.CWD, path)
 	}
-	if real, err := filepath.EvalSymlinks(path); err == nil {
+	if real, err := w.pathResolver().evalSymlinks(path); err == nil {
 		path = real
 	}
 	return path, nil

internal/etch/git_object_store_test.go

@@ -13,7 +13,7 @@ func TestPlanAndDryRunUseInjectedTempStore(t *testing.T) {
 	tempRoot := t.TempDir()
 	temp := &recordingTempStore{root: tempRoot}
 
-	w, err := openWorkspaceAtWithDeps(dir, false, workspaceDeps{
+	w, err := openWorkspaceAt(dir, false, workspaceDeps{
 		git:      realGitRunner{},
 		worktree: osWorkingTreeFS{},
 		temp:     temp,

internal/etch/script_source.go

@@ -0,0 +1,56 @@
+package etch
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+)
+
+// scriptSource is the IO boundary for etch scripts. The parser works on bytes;
+// this source keeps command-line script loading injectable and separate from
+// CLI argument handling.
+type scriptSource interface {
+	readFile(path string) ([]byte, error)
+	readStdin() ([]byte, error)
+}
+
+type osScriptSource struct{}
+
+func ParseScriptAt(cwd, path string) ([]Statement, error) {
+	return parseScriptAt(cwd, path, osScriptSource{})
+}
+
+func parseScriptAt(cwd, path string, source scriptSource) ([]Statement, error) {
+	if source == nil {
+		source = osScriptSource{}
+	}
+	var data []byte
+	var err error
+	name := path
+	if path == "-" {
+		name = "<stdin>"
+		data, err = source.readStdin()
+	} else {
+		readPath := path
+		if cwd != "" && !filepath.IsAbs(readPath) {
+			readPath = filepath.Join(cwd, readPath)
+		}
+		data, err = source.readFile(readPath)
+	}
+	if err != nil {
+		return nil, failf("%s", err)
+	}
+	return ParseScriptBytes(name, data)
+}
+
+func (osScriptSource) readFile(path string) ([]byte, error) {
+	return os.ReadFile(path)
+}
+
+func (osScriptSource) readStdin() ([]byte, error) {
+	return readStdin()
+}
+
+var readStdin = func() ([]byte, error) {
+	return io.ReadAll(os.Stdin)
+}

internal/etch/script_source_test.go

@@ -0,0 +1,65 @@
+package etch
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestParseScriptAtUsesInjectedScriptSource(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "ops.etch")
+	source := &fakeScriptSource{
+		files: map[string][]byte{
+			path: []byte("set state.json status complete\n"),
+		},
+	}
+
+	stmts, err := parseScriptAt(dir, "ops.etch", source)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := source.reads, []string{path}; len(got) != len(want) || got[0] != want[0] {
+		t.Fatalf("script reads = %v, want %v", got, want)
+	}
+	if len(stmts) != 1 || stmts[0].Loc.Name != "ops.etch" || stmts[0].Tokens[0] != "set" {
+		t.Fatalf("statements = %#v", stmts)
+	}
+}
+
+func TestParseScriptAtUsesInjectedStdin(t *testing.T) {
+	source := &fakeScriptSource{stdin: []byte("set stdin.json x 1\n")}
+
+	stmts, err := parseScriptAt("ignored", "-", source)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !source.stdinRead {
+		t.Fatal("stdin was not read")
+	}
+	if len(stmts) != 1 || stmts[0].Loc.Name != "<stdin>" || stmts[0].Tokens[1] != "stdin.json" {
+		t.Fatalf("statements = %#v", stmts)
+	}
+}
+
+type fakeScriptSource struct {
+	files     map[string][]byte
+	stdin     []byte
+	reads     []string
+	stdinRead bool
+}
+
+func (s *fakeScriptSource) readFile(path string) ([]byte, error) {
+	s.reads = append(s.reads, path)
+	b, ok := s.files[path]
+	if !ok {
+		return nil, fmt.Errorf("%w: %s", os.ErrNotExist, path)
+	}
+	return append([]byte(nil), b...), nil
+}
+
+func (s *fakeScriptSource) readStdin() ([]byte, error) {
+	s.stdinRead = true
+	return append([]byte(nil), s.stdin...), nil
+}

internal/etch/workspace_paths.go

@@ -0,0 +1,36 @@
+package etch
+
+import (
+	"os"
+	"path/filepath"
+)
+
+// workspacePaths is the boundary for process-dependent path resolution. The
+// rest of Workspace can still use filepath's pure lexical operations directly,
+// but current-directory and symlink resolution go through this interface.
+type workspacePaths interface {
+	getwd() (string, error)
+	abs(path string) (string, error)
+	evalSymlinks(path string) (string, error)
+}
+
+type osWorkspacePaths struct{}
+
+func (w *Workspace) pathResolver() workspacePaths {
+	if w.paths != nil {
+		return w.paths
+	}
+	return osWorkspacePaths{}
+}
+
+func (osWorkspacePaths) getwd() (string, error) {
+	return os.Getwd()
+}
+
+func (osWorkspacePaths) abs(path string) (string, error) {
+	return filepath.Abs(path)
+}
+
+func (osWorkspacePaths) evalSymlinks(path string) (string, error) {
+	return filepath.EvalSymlinks(path)
+}