refactor: capture time.Now() once at shell start, add NowSafe() guard, fix fuzz test context leaks

SHELL_FEATURES.md

@@ -112,6 +112,10 @@ Blocked features are rejected before execution with exit code 2.
 - ❌ No automatic inheritance from the host process
 - ❌ `export`, `readonly` are blocked
 
+## Intentional Divergences from Bash
+
+- **Time reference for `find -mmin`/`-mtime` and `ls -l`**: rshell captures `time.Now()` once at the start of each `Run()` call and shares it across all builtins in that run. Bash evaluates each command against its own invocation time. In practice this only matters for long-running scripts (e.g. `sleep 61; find . -mmin -1`) where the reference time drifts from the actual command start. Short-lived AI agent scripts are unaffected.
+
 ## Appendix
 
-Formating: In each category, supported features should be listed first, and the most useful ones first.
+Formatting: In each category, supported features should be listed first, and the most useful ones first.

allowedsymbols/symbols_interp.go

@@ -59,6 +59,7 @@ var interpAllowedSymbols = []string{
 	"sync.Once",            // ensures a function runs exactly once; concurrency primitive, no I/O.
 	"sync.WaitGroup",       // waits for goroutines to finish; concurrency primitive, no I/O.
 	"time.Now",             // returns current time; read-only, no mutation.
+	"time.Time",            // time value type; pure data, no side effects.
 
 	// --- mvdan.cc/sh/v3/expand --- (shell word expansion library)
 

builtins/builtins.go

@@ -121,10 +121,22 @@ type CallContext struct {
 	// PortableErr normalizes an OS error to a POSIX-style message.
 	PortableErr func(err error) string
 
-	// Now returns the current time. Builtins should use this instead of
-	// calling time.Now() directly, so the time source is consistent and
-	// testable.
-	Now func() time.Time
+	// Now is the time captured at the start of each Run() call. Builtins
+	// should use this instead of calling time.Now() directly, so the time
+	// source is consistent across all commands in a single run.
+	//
+	// Note: this means all builtins within one Run() share the same reference
+	// time, whereas bash evaluates each command against its own invocation
+	// time. This is an intentional trade-off for consistency within a script
+	// run.
+	//
+	// Run() always sets this before dispatching any builtin; Reset() clears
+	// it, so it is always re-set by the next Run() call. The zero value
+	// (time.Time{}) is reserved as the unset sentinel; callers constructing
+	// CallContext directly (e.g. in tests) must set this to a non-zero value
+	// before invoking builtins that use time predicates (find -mmin/-mtime,
+	// ls -l).
+	Now time.Time
 
 	// FileIdentity extracts canonical file identity from FileInfo.
 	// On Unix: dev+inode from Stat_t. On Windows: volume serial + file index

builtins/find/find.go

@@ -207,9 +207,7 @@ optLoop:
 		}
 	}
 
-	// Capture invocation time once so -mtime/-mmin predicates use a
-	// consistent reference across all root paths (matches GNU find).
-	now := callCtx.Now()
+	now := callCtx.Now
 
 	// GNU find treats a missing -newer reference as a fatal argument error
 	// and produces no result set, so skip the walk entirely.

builtins/find/now_test.go

@@ -11,7 +11,6 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -21,11 +20,13 @@ import (
 	"github.com/DataDog/rshell/builtins"
 )
 
-// TestNowCalledOnce verifies that find captures the invocation timestamp
-// once in run(), not per root path. GNU find evaluates -mtime/-mmin
-// relative to a single invocation time, so multi-path invocations must
-// use a consistent reference.
-func TestNowCalledOnce(t *testing.T) {
+// TestNowConsistentAcrossRoots verifies that find with multiple root paths
+// finds files in all roots when using a shared time reference from
+// CallContext.Now. The single-capture invariant is now structurally enforced
+// by the time.Time value field (there is no function to call multiple times),
+// so this test acts as a regression guard for multi-root traversal with a
+// time predicate.
+func TestNowConsistentAcrossRoots(t *testing.T) {
 	// Create two directories with one file each.
 	tmp := t.TempDir()
 	dir1 := filepath.Join(tmp, "a")
@@ -35,17 +36,11 @@ func TestNowCalledOnce(t *testing.T) {
 	require.NoError(t, os.WriteFile(filepath.Join(dir1, "f1.txt"), []byte("x"), 0644))
 	require.NoError(t, os.WriteFile(filepath.Join(dir2, "f2.txt"), []byte("y"), 0644))
 
-	var nowCalls atomic.Int32
-	fixedNow := time.Now()
-
 	var stdout, stderr bytes.Buffer
 	callCtx := &builtins.CallContext{
 		Stdout: &stdout,
 		Stderr: &stderr,
-		Now: func() time.Time {
-			nowCalls.Add(1)
-			return fixedNow
-		},
+		Now:    time.Now(),
 		LstatFile: func(_ context.Context, path string) (fs.FileInfo, error) {
 			return os.Lstat(filepath.Join(tmp, path))
 		},
@@ -71,8 +66,63 @@ func TestNowCalledOnce(t *testing.T) {
 	result := run(context.Background(), callCtx, []string{"a", "b", "-mmin", "-60"})
 
 	assert.Equal(t, uint8(0), result.Code, "find should succeed")
-	assert.Equal(t, int32(1), nowCalls.Load(),
-		"Now() should be called exactly once per find invocation, not per root path")
 	assert.Contains(t, stdout.String(), "f1.txt")
 	assert.Contains(t, stdout.String(), "f2.txt")
 }
+
+// TestNowFromCallContextIsUsed verifies that find actually uses the Now value
+// from CallContext for predicate evaluation. A fixed timestamp far in the
+// future is supplied; files created right now should appear very old relative
+// to that future Now, so they should match +1 (older than 1 minute) and not
+// match -1 (newer than 1 minute).
+func TestNowFromCallContextIsUsed(t *testing.T) {
+	tmp := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(tmp, "fresh.txt"), []byte("x"), 0644))
+
+	var stdout, stderr bytes.Buffer
+	// Use a timestamp 10 years in the future. From that reference point the
+	// fresh file was created "10 years ago", so diff = futureNow - mtime ≈ 10yr.
+	// It should match +1 (older than 1 minute) but not -1 (newer than 1 minute).
+	futureNow := time.Now().Add(10 * 365 * 24 * time.Hour)
+	callCtx := &builtins.CallContext{
+		Stdout: &stdout,
+		Stderr: &stderr,
+		Now:    futureNow,
+		LstatFile: func(_ context.Context, path string) (fs.FileInfo, error) {
+			return os.Lstat(filepath.Join(tmp, path))
+		},
+		StatFile: func(_ context.Context, path string) (fs.FileInfo, error) {
+			return os.Stat(filepath.Join(tmp, path))
+		},
+		OpenDir: func(_ context.Context, path string) (fs.ReadDirFile, error) {
+			return os.Open(filepath.Join(tmp, path))
+		},
+		IsDirEmpty: func(_ context.Context, path string) (bool, error) {
+			entries, err := os.ReadDir(filepath.Join(tmp, path))
+			if err != nil {
+				return false, err
+			}
+			return len(entries) == 0, nil
+		},
+		PortableErr: func(err error) string {
+			return err.Error()
+		},
+	}
+
+	// A fresh file should match +1 (older than 1 minute) when Now is 10 years
+	// in the future, proving that CallContext.Now is used for evaluation.
+	result := run(context.Background(), callCtx, []string{".", "-name", "fresh.txt", "-mmin", "+1"})
+	assert.Equal(t, uint8(0), result.Code, "find should succeed")
+	assert.Contains(t, stdout.String(), "fresh.txt",
+		"fresh file should match -mmin +1 when CallContext.Now is 10 years in the future")
+	assert.Empty(t, stderr.String())
+
+	// The same file should NOT match -1 (newer than 1 minute) under the same Now.
+	stdout.Reset()
+	stderr.Reset()
+	result = run(context.Background(), callCtx, []string{".", "-name", "fresh.txt", "-mmin", "-1"})
+	assert.Equal(t, uint8(0), result.Code, "find should succeed")
+	assert.NotContains(t, stdout.String(), "fresh.txt",
+		"fresh file should not match -mmin -1 when CallContext.Now is 10 years in the future")
+	assert.Empty(t, stderr.String())
+}

builtins/ls/ls.go

@@ -137,7 +137,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc {
 			return builtins.Result{}
 		}
 
-		now := callCtx.Now()
+		now := callCtx.Now
 
 		// Determine the effective sort mode. When both -S and -t are given,
 		// the last one specified wins, matching GNU ls behaviour.

builtins/tests/cat/cat_differential_fuzz_test.go

@@ -86,13 +86,17 @@ func FuzzCatDifferential(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		// Use context.Background() (not t.Context()) so the fuzz engine's
+		// cancellation does not kill the command mid-run; each iteration still
+		// enforces its own 5 s deadline.
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		rshellOut, rshellErr, rshellCode := cmdRunCtx(ctx, t, "cat input.txt", dir)
+		cancel()
 
-		// If the fuzz engine cancelled us (fuzztime expired), bail out
-		// without comparing — partial output would cause false failures.
+		// If the fuzz engine's budget expired (t.Context(), not the per-command
+		// context above), bail out without comparing — partial output would cause
+		// false failures.
 		if t.Context().Err() != nil {
 			return
 		}

builtins/tests/cat/cat_fuzz_test.go

@@ -75,10 +75,10 @@ func FuzzCat(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		stdout, _, code := cmdRunCtx(ctx, t, "cat input.txt", dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("unexpected exit code %d", code)
 		}
@@ -126,10 +126,10 @@ func FuzzCatNumberLines(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtx(ctx, t, "cat -n input.txt", dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cat -n unexpected exit code %d", code)
 		}
@@ -188,10 +188,10 @@ func FuzzCatDisplayFlags(f *testing.F) {
 			flags += " -T"
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtx(ctx, t, "cat"+flags+" input.bin", dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cat%s unexpected exit code %d", flags, code)
 		}
@@ -226,10 +226,10 @@ func FuzzCatStdin(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		stdout, _, code := cmdRunCtx(ctx, t, "cat < stdin.txt", dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cat stdin unexpected exit code %d", code)
 		}

builtins/tests/cut/cut_fuzz_test.go

@@ -97,10 +97,10 @@ func FuzzCutFields(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtxFuzz(ctx, t, fmt.Sprintf("cut -f %s input.txt", fieldSpec), dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cut -f %s unexpected exit code %d", fieldSpec, code)
 		}
@@ -165,10 +165,10 @@ func FuzzCutBytes(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtxFuzz(ctx, t, fmt.Sprintf("cut -b %s input.txt", byteSpec), dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cut -b %s unexpected exit code %d", byteSpec, code)
 		}
@@ -231,11 +231,11 @@ func FuzzCutDelimiter(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		script := fmt.Sprintf("cut -d '%s' -f %s input.txt", delim, fieldSpec)
 		_, _, code := cmdRunCtxFuzz(ctx, t, script, dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cut -d '%s' -f %s unexpected exit code %d", delim, fieldSpec, code)
 		}
@@ -290,10 +290,10 @@ func FuzzCutComplement(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtxFuzz(ctx, t, fmt.Sprintf("cut --complement -b %s input.txt", byteSpec), dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cut --complement -b %s unexpected exit code %d", byteSpec, code)
 		}
@@ -331,10 +331,10 @@ func FuzzCutStdin(f *testing.F) {
 			t.Fatal(err)
 		}
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := cmdRunCtxFuzz(ctx, t, "cut -f 1 < stdin.txt", dir)
+		cancel()
 		if code != 0 && code != 1 {
 			t.Errorf("cut stdin unexpected exit code %d", code)
 		}

builtins/tests/echo/echo_fuzz_test.go

@@ -60,10 +60,10 @@ func FuzzEcho(f *testing.F) {
 		dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter)
 		defer cleanup()
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := fuzzRunCtx(ctx, t, "echo '"+arg+"'", dir)
+		cancel()
 		if code != 0 {
 			t.Errorf("echo unexpected exit code %d", code)
 		}
@@ -132,10 +132,10 @@ func FuzzEchoEscapes(f *testing.F) {
 		dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter)
 		defer cleanup()
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := fuzzRunCtx(ctx, t, "echo -e '"+arg+"'", dir)
+		cancel()
 		if code != 0 {
 			t.Errorf("echo -e unexpected exit code %d", code)
 		}
@@ -188,10 +188,10 @@ func FuzzEchoFlagInteraction(f *testing.F) {
 		dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter)
 		defer cleanup()
 
-		ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
-		defer cancel()
-
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // safety net if t.Fatal fires before explicit cancel
 		_, _, code := fuzzRunCtx(ctx, t, "echo"+flags+" '"+arg+"'", dir)
+		cancel()
 		if code != 0 {
 			t.Errorf("echo%s unexpected exit code %d", flags, code)
 		}