From 0ae6bccc3e777caba3790e2ca78fb424100ccd17 Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:34:18 +0800 Subject: [PATCH 1/2] [feat] CLI-02/C-4: --log-file routes the script log module to a file (STAR-19) Add --log-file : when a script uses the starlet log module, its output is captured to that file at the interpreter level, so a host can collect a run's logs in one place. - starbox routes the log module through the box logger (module.go: it uses slog.NewModule(userLog) when a logger is set), so BuildBox builds a file-backed zap logger (console encoder, ISO8601 time, all levels) and SetLogger's it when --log-file is given. - The parent directory is created if needed; runs append. The logger is memoized per path so the web mode (a BuildBox per request) shares one open file instead of leaking a descriptor each request. Tests route a script's log.info to a temp --log-file (nested dir) and assert the message + level land in the file. README documents it. Coverage 75.9%; Docker golang:1.22 green. --- README.md | 20 +++++++++++++++++++ cli/args.go | 3 +++ cli/box.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ cli/wiring_test.go | 27 +++++++++++++++++++++++++ 4 files changed, 99 insertions(+) diff --git a/README.md b/README.md index 3603033..5f0f54c 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ Usage of ./starcli: -I, --include string include path for Starlark code to load modules from (default ".") -i, --interactive enter interactive mode after executing -l, --log string log level: debug, info, warn, error, dpanic, panic, fatal (default "info") + --log-file string append the script's log module output to this file --max-output uint max top-level output entries per run (0=unlimited) --max-steps uint max Starlark execution steps per run, guards runaway loops (0=unlimited) -m, --module strings allowed modules to preload and load (default [args,atom,base64,cmd,csv,email,file,go_idiomatic,gum,hashlib,http,json,llm,log,markdown,math,net,path,random,re,regex,runtime,s3,serial,sqlite,stats,string,struct,sys,time,web]) @@ -201,6 +202,25 @@ $ ./starcli greet.star -- --name Kevin --count 3 --shout in.txt Kevin 3 True in.txt ``` +#### Capture Logs to a File + +When a script uses the `log` module, `--log-file` routes all of its output to a +file at the interpreter level (the parent directory is created if needed, and +runs append): + +```python +load("log", "info", "warn") +info("starting up") +warn("careful now") +``` + +```bash +$ ./starcli --log-file run.log job.star +$ cat run.log +2026-06-21T17:32:07.373+0800 info starting up +2026-06-21T17:32:07.373+0800 warn careful now +``` + #### Read Piped Input The `sys` module reads piped **data** from standard input (the script itself diff --git a/cli/args.go b/cli/args.go index 3943a3b..7fbffc0 100644 --- a/cli/args.go +++ b/cli/args.go @@ -29,6 +29,7 @@ type Args struct { AllowFS bool AllowCmd bool Check bool + LogFile string } // ParseArgs parses command line arguments and returns the Args object. @@ -54,6 +55,7 @@ func ParseArgs() *Args { flag.BoolVar(&args.AllowFS, "allow-fs", false, "widen a restrictive tier with filesystem modules (file, path)") flag.BoolVar(&args.AllowCmd, "allow-cmd", false, "widen a restrictive tier with the cmd module (host command execution)") flag.BoolVar(&args.Check, "check", false, "syntax/resolve check the script (-c or file) without running it") + flag.StringVar(&args.LogFile, "log-file", "", "append the script's log module output to this file") flag.Parse() // Capability tier resolution: an explicit --caps wins; otherwise fall back @@ -78,6 +80,7 @@ func (a *Args) BasicBoxOpts() *BoxOpts { printerName: a.OutputPrinter, recursion: a.AllowRecursion, globalReassign: a.AllowGlobalReassign, + logFile: a.LogFile, maxSteps: a.MaxSteps, maxOutput: a.MaxOutput, caps: a.Caps, diff --git a/cli/box.go b/cli/box.go index 3fc808d..16a46c5 100644 --- a/cli/box.go +++ b/cli/box.go @@ -3,9 +3,13 @@ package cli import ( "fmt" "os" + "path/filepath" + "sync" "github.com/1set/gut/ystring" "github.com/1set/starbox" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) type scenarioCode uint @@ -27,6 +31,7 @@ type BoxOpts struct { printerName string recursion bool globalReassign bool + logFile string // if set, the script's log module writes here maxSteps uint64 // per-run Starlark step budget; 0 = unlimited maxOutput uint // per-run top-level output entry cap; 0 = unlimited caps string // capability tier: safe (default) / network / full @@ -54,6 +59,18 @@ func BuildBox(opts *BoxOpts) (*starbox.Starbox, error) { policy := starbox.Policy{Modules: starbox.ModuleAllow{Names: grant.allowedModules(getDefaultModules())}} box = starbox.NewWithPolicy(opts.name, policy) } + + // Route the script's `log` module output to a file when requested (C-4): + // starbox uses the box logger for the log module, so a file-backed logger + // captures every log.* call at the interpreter level. + if opts.logFile != "" { + lg, err := fileLogger(opts.logFile) + if err != nil { + return nil, err + } + box.SetLogger(lg) + } + if ystring.IsNotBlank(opts.includePath) { box.SetFS(os.DirFS(opts.includePath)) } @@ -91,3 +108,35 @@ func BuildBox(opts *BoxOpts) (*starbox.Starbox, error) { } return box, nil } + +var ( + logFileMu sync.Mutex + logFileLoggers = map[string]*zap.SugaredLogger{} +) + +// fileLogger returns a zap logger that appends every level to path, memoized so +// repeated BuildBox calls (e.g. one per web request) share a single open file +// instead of leaking a descriptor each time. The parent directory is created if +// needed; writes are synchronous, so no explicit flush is required. +func fileLogger(path string) (*zap.SugaredLogger, error) { + logFileMu.Lock() + defer logFileMu.Unlock() + if lg, ok := logFileLoggers[path]; ok { + return lg, nil + } + if dir := filepath.Dir(path); dir != "" && dir != "." { + if err := os.MkdirAll(dir, 0o755); err != nil { + return nil, fmt.Errorf("log file: %w", err) + } + } + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return nil, fmt.Errorf("log file: %w", err) + } + encCfg := zap.NewProductionEncoderConfig() + encCfg.EncodeTime = zapcore.ISO8601TimeEncoder + core := zapcore.NewCore(zapcore.NewConsoleEncoder(encCfg), zapcore.AddSync(f), zapcore.DebugLevel) + lg := zap.New(core).Sugar() + logFileLoggers[path] = lg + return lg, nil +} diff --git a/cli/wiring_test.go b/cli/wiring_test.go index fe740d3..c2d296d 100644 --- a/cli/wiring_test.go +++ b/cli/wiring_test.go @@ -119,6 +119,33 @@ func TestBuildBox_BudgetsApplied(t *testing.T) { } } +// --- log-file routing (--log-file) ---------------------------------------- + +func TestProcess_LogFile(t *testing.T) { + // A script's log module output is routed to the --log-file at the + // interpreter level (nested directory auto-created). + logPath := filepath.Join(t.TempDir(), "logs", "run.log") + a := baseArgs() + a.LogFile = logPath + a.CodeContent = `load("log", "info"); info("captured-line")` + + var code int + captureStd(t, func() { code = Process(a) }) + if code != 0 { + t.Fatalf("log-file run: exit=%d want 0", code) + } + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read log file: %v", err) + } + if !strings.Contains(string(data), "captured-line") { + t.Errorf("log file missing the message: %q", string(data)) + } + if !strings.Contains(string(data), "info") { + t.Errorf("log file missing the level: %q", string(data)) + } +} + // --- check mode (--check) ------------------------------------------------- func TestProcess_Check(t *testing.T) { From cf9f18b3885dd35f12bd38aa449d390d0beb6733 Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:39:35 +0800 Subject: [PATCH 2/2] [fix] close memoized log files so Windows TempDir cleanup can remove them The fileLogger memo holds the log file open for the process lifetime; on Windows an open file cannot be deleted, so TestProcess_LogFile's t.TempDir cleanup failed there. Track the *os.File handles and add closeLogFiles(); the test registers it via t.Cleanup (after TempDir, so LIFO runs it first) to release the handle before RemoveAll. --- cli/box.go | 17 +++++++++++++++++ cli/wiring_test.go | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/cli/box.go b/cli/box.go index 16a46c5..6186f75 100644 --- a/cli/box.go +++ b/cli/box.go @@ -112,6 +112,7 @@ func BuildBox(opts *BoxOpts) (*starbox.Starbox, error) { var ( logFileMu sync.Mutex logFileLoggers = map[string]*zap.SugaredLogger{} + logFileHandles = map[string]*os.File{} ) // fileLogger returns a zap logger that appends every level to path, memoized so @@ -138,5 +139,21 @@ func fileLogger(path string) (*zap.SugaredLogger, error) { core := zapcore.NewCore(zapcore.NewConsoleEncoder(encCfg), zapcore.AddSync(f), zapcore.DebugLevel) lg := zap.New(core).Sugar() logFileLoggers[path] = lg + logFileHandles[path] = f return lg, nil } + +// closeLogFiles flushes and closes every memoized log file. The process holds +// them open for its lifetime (the OS closes them on exit), so this exists for +// tests, which must release the handle before the temp dir can be removed +// (notably on Windows, where an open file cannot be deleted). +func closeLogFiles() { + logFileMu.Lock() + defer logFileMu.Unlock() + for path, f := range logFileHandles { + _ = f.Sync() + _ = f.Close() + delete(logFileHandles, path) + delete(logFileLoggers, path) + } +} diff --git a/cli/wiring_test.go b/cli/wiring_test.go index c2d296d..8950638 100644 --- a/cli/wiring_test.go +++ b/cli/wiring_test.go @@ -124,7 +124,11 @@ func TestBuildBox_BudgetsApplied(t *testing.T) { func TestProcess_LogFile(t *testing.T) { // A script's log module output is routed to the --log-file at the // interpreter level (nested directory auto-created). - logPath := filepath.Join(t.TempDir(), "logs", "run.log") + dir := t.TempDir() + // Registered after TempDir so it runs first (LIFO): release the open handle + // before TempDir's RemoveAll, which on Windows cannot delete an open file. + t.Cleanup(closeLogFiles) + logPath := filepath.Join(dir, "logs", "run.log") a := baseArgs() a.LogFile = logPath a.CodeContent = `load("log", "info"); info("captured-line")`