From 9e296295c05e4162db9dca9dcf6c05f5d2795e21 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:07:15 +0000 Subject: [PATCH] Let a long step's exec runner be bound by its context alone (issue 1497), part 8 of 8 --- go/forgekey/runner.go | 26 ++++++++++++++++++----- go/forgekey/runner_test.go | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 go/forgekey/runner_test.go diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index 9cc08e1..bc8628c 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -23,14 +23,30 @@ type Ran struct { // Runner runs one command with extra environment words, so every tool can be tested without ssh. type Runner func(ctx context.Context, env []string, name string, args ...string) Ran -// CallTimeout bounds one command: below the runtime's thirty-second call limit. -const CallTimeout = 20 * time.Second +// CallTimeout bounds one command of ExecRunner: below the runtime's thirty-second call limit. A variable so a +// test can lower it. +var CallTimeout = 20 * time.Second // ExecRunner runs a command on this machine, with no terminal and nothing on its stdin, bounded by // CallTimeout. func ExecRunner(ctx context.Context, env []string, name string, args ...string) Ran { - ctx, cancel := context.WithTimeout(ctx, CallTimeout) - defer cancel() + return ExecRunnerFor(CallTimeout)(ctx, env, name, args...) +} + +// ExecRunnerFor is ExecRunner with its own bound (issue 1497): a build module's long step passes 0, so only the +// step's own context ends a command, and a fetch may outlast a tool call's thirty seconds. +func ExecRunnerFor(bound time.Duration) Runner { + return func(ctx context.Context, env []string, name string, args ...string) Ran { + return execRun(ctx, bound, env, name, args...) + } +} + +func execRun(ctx context.Context, bound time.Duration, env []string, name string, args ...string) Ran { + if bound > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, bound) + defer cancel() + } cmd := exec.CommandContext(ctx, name, args...) cmd.Env = append(os.Environ(), env...) var out, errb bytes.Buffer @@ -40,7 +56,7 @@ func ExecRunner(ctx context.Context, env []string, name string, args ...string) var exitErr *exec.ExitError switch { case errors.Is(ctx.Err(), context.DeadlineExceeded): - r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(CallTimeout/time.Second)) + r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(bound/time.Second)) case errors.Is(err, exec.ErrNotFound): r.Status, r.Err = 127, "ENOENT" case errors.As(err, &exitErr): diff --git a/go/forgekey/runner_test.go b/go/forgekey/runner_test.go new file mode 100644 index 0000000..8b94fd2 --- /dev/null +++ b/go/forgekey/runner_test.go @@ -0,0 +1,42 @@ +package forgekey + +import ( + "context" + "testing" + "time" +) + +func lowered(t *testing.T) { + was := CallTimeout + CallTimeout = 50 * time.Millisecond + t.Cleanup(func() { CallTimeout = was }) +} + +// 1497/AC1: ExecRunnerFor(0), no deadline: a command longer than the lowered CallTimeout is not cut. +func TestExecRunnerForZeroIsNotCutByCallTimeout(t *testing.T) { + lowered(t) + r := ExecRunnerFor(0)(context.Background(), nil, "sleep", "0.2") + if r.Status != 0 || r.Err != "" { + t.Errorf("ran = %+v, want status 0", r) + } +} + +// 1497/AC2: ExecRunner is unchanged: cut at CallTimeout with status 124. +func TestExecRunnerIsStillCutAtCallTimeout(t *testing.T) { + lowered(t) + r := ExecRunner(context.Background(), nil, "sleep", "0.2") + if r.Status != 124 { + t.Errorf("ran = %+v, want status 124", r) + } +} + +// 1497/AC3: ExecRunnerFor(0) is cut when its context is cancelled. +func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + began := time.Now() + r := ExecRunnerFor(0)(ctx, nil, "sleep", "2") + if r.Status == 0 || time.Since(began) > time.Second { + t.Errorf("ran = %+v after %v, want it cut", r, time.Since(began)) + } +}