From 9b536f08c6198c5076ce1ac63e47b7ee69b49cd6 Mon Sep 17 00:00:00 2001 From: Virgil Date: Fri, 3 Apr 2026 23:56:56 +0000 Subject: [PATCH] feat(exec): require command context --- exec/exec.go | 34 +++++++++++++++++++++------------- exec/exec_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/exec/exec.go b/exec/exec.go index 00b1d69..cc6f28d 100644 --- a/exec/exec.go +++ b/exec/exec.go @@ -12,6 +12,9 @@ import ( coreerr "dappco.re/go/core/log" ) +// ErrCommandContextRequired is returned when a command is created without a context. +var ErrCommandContextRequired = coreerr.E("", "exec: command context is required", nil) + // Options configuration for command execution type Options struct { Dir string @@ -87,7 +90,9 @@ func (c *Cmd) WithBackground(background bool) *Cmd { // Start launches the command. func (c *Cmd) Start() error { - c.prepare() + if err := c.prepare(); err != nil { + return err + } c.logDebug("executing command") if err := c.cmd.Start(); err != nil { @@ -112,7 +117,9 @@ func (c *Cmd) Run() error { return c.Start() } - c.prepare() + if err := c.prepare(); err != nil { + return err + } c.logDebug("executing command") if err := c.cmd.Run(); err != nil { @@ -129,7 +136,9 @@ func (c *Cmd) Output() ([]byte, error) { return nil, coreerr.E("Cmd.Output", "background execution is incompatible with Output", nil) } - c.prepare() + if err := c.prepare(); err != nil { + return nil, err + } c.logDebug("executing command") out, err := c.cmd.Output() @@ -147,7 +156,9 @@ func (c *Cmd) CombinedOutput() ([]byte, error) { return nil, coreerr.E("Cmd.CombinedOutput", "background execution is incompatible with CombinedOutput", nil) } - c.prepare() + if err := c.prepare(); err != nil { + return nil, err + } c.logDebug("executing command") out, err := c.cmd.CombinedOutput() @@ -159,17 +170,13 @@ func (c *Cmd) CombinedOutput() ([]byte, error) { return out, nil } -func (c *Cmd) prepare() { - if c.ctx != nil { - c.cmd = exec.CommandContext(c.ctx, c.name, c.args...) - } else { - // Should we enforce context? The issue says "Enforce context usage". - // For now, let's allow nil but log a warning if we had a logger? - // Or strictly panic/error? - // Let's fallback to Background for now but maybe strict later. - c.cmd = exec.Command(c.name, c.args...) +func (c *Cmd) prepare() error { + if c.ctx == nil { + return coreerr.E("Cmd.prepare", "exec: command context is required", ErrCommandContextRequired) } + c.cmd = exec.CommandContext(c.ctx, c.name, c.args...) + c.cmd.Dir = c.opts.Dir if len(c.opts.Env) > 0 { c.cmd.Env = append(os.Environ(), c.opts.Env...) @@ -178,6 +185,7 @@ func (c *Cmd) prepare() { c.cmd.Stdin = c.opts.Stdin c.cmd.Stdout = c.opts.Stdout c.cmd.Stderr = c.opts.Stderr + return nil } // RunQuiet executes the command suppressing stdout unless there is an error. diff --git a/exec/exec_test.go b/exec/exec_test.go index f7d8cb9..8519468 100644 --- a/exec/exec_test.go +++ b/exec/exec_test.go @@ -10,6 +10,8 @@ import ( "time" "dappco.re/go/core/process/exec" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // mockLogger captures log calls for testing @@ -231,6 +233,32 @@ func TestCommand_Run_Background(t *testing.T) { } } +func TestCommand_NilContextRejected(t *testing.T) { + t.Run("start", func(t *testing.T) { + err := exec.Command(nil, "echo", "test").Start() + require.Error(t, err) + assert.ErrorIs(t, err, exec.ErrCommandContextRequired) + }) + + t.Run("run", func(t *testing.T) { + err := exec.Command(nil, "echo", "test").Run() + require.Error(t, err) + assert.ErrorIs(t, err, exec.ErrCommandContextRequired) + }) + + t.Run("output", func(t *testing.T) { + _, err := exec.Command(nil, "echo", "test").Output() + require.Error(t, err) + assert.ErrorIs(t, err, exec.ErrCommandContextRequired) + }) + + t.Run("combined output", func(t *testing.T) { + _, err := exec.Command(nil, "echo", "test").CombinedOutput() + require.Error(t, err) + assert.ErrorIs(t, err, exec.ErrCommandContextRequired) + }) +} + func TestCommand_Output_BackgroundRejected(t *testing.T) { ctx := context.Background()