From 33a246cd1327175cac75914c6d94d8a54a847e3a Mon Sep 17 00:00:00 2001 From: mixeme Date: Sun, 26 Jul 2026 13:57:03 +0300 Subject: [PATCH] feat: make global default timeout 0 (infinite) instead of required 30s DefaultTimeoutSeconds now means "no timeout" when 0/empty, and that is the new default, rather than an invalid config forcing a positive value. runner.RunJob avoids context.WithTimeout with a zero duration (which would expire immediately) and instead runs on a plain cancelable context when no timeout is configured. Per-job TimeoutSeconds inherit semantics are unchanged. --- docs/ARCHITECTURE.md | 18 ++++++++++-------- docs/STANDARDS.md | 5 +++-- src/app/format.go | 5 ++++- src/app/format_test.go | 3 +++ src/app/operations.go | 4 ++-- src/app/operations_test.go | 2 +- src/app/run.go | 3 ++- src/app/run_test.go | 5 +++++ src/domain/config.go | 5 +++-- src/domain/job.go | 1 + src/runner/runner.go | 13 +++++++++++-- src/runner/runner_test.go | 25 +++++++++++++++++++++++++ src/storage/store_test.go | 4 ++-- src/ui/settings_view.go | 5 +++-- 14 files changed, 75 insertions(+), 23 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a2aa8eb..1a12c3e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -133,14 +133,16 @@ in flight increments `JobRuntime.PendingRuns`. When the current run finishes, `domain.Job` carries a `TimeoutSeconds` field (`json:"timeout_seconds,omitempty"`), following the same inherit pattern as the overlap policy. `0` means inherit the -global `Config.DefaultTimeoutSeconds` (default **30**); a positive value overrides -it for that job alone. `app.Service.effectiveTimeout` resolves the effective -duration under `mu` and `startRunLocked` snapshots it into `runEnv.timeout`. -`runner.RunJob(ctx, job, trigger, logsDir, timeout)` takes the resolved duration -as an argument, so the runner stays ignorant of the global config: it applies the -timeout via `context.WithTimeout` and reports `Timed out after ` on -expiry. `StartOnly` jobs run on the untimed context and so measure launch latency -only, unaffected by the run timeout. +global `Config.DefaultTimeoutSeconds` (default **0**, i.e. no timeout); a +positive value overrides it for that job alone. `app.Service.effectiveTimeout` +resolves the effective duration under `mu` and `startRunLocked` snapshots it into +`runEnv.timeout`. `runner.RunJob(ctx, job, trigger, logsDir, timeout)` takes the +resolved duration as an argument, so the runner stays ignorant of the global +config: a positive duration applies the timeout via `context.WithTimeout` and +reports `Timed out after ` on expiry; a non-positive duration runs +without a deadline, bounded only by `ctx` (app shutdown). `StartOnly` jobs run on +the untimed context and so measure launch latency only, unaffected by the run +timeout. ### Run-time statistics diff --git a/docs/STANDARDS.md b/docs/STANDARDS.md index 6b6b8ef..2d6fc7d 100644 --- a/docs/STANDARDS.md +++ b/docs/STANDARDS.md @@ -17,8 +17,9 @@ in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md). - `RunNow` is allowed during global pause and for disabled jobs. - Sequential mode runs jobs FIFO by order in `jobs.json`. - Scheduler tick is 1s — sub-second `@every` intervals are not supported. -- Command timeout defaults to 30s globally and is overridable per job - (`Job.TimeoutSeconds`, 0 = inherit `Config.DefaultTimeoutSeconds`). +- Command timeout defaults to no timeout globally (`Config.DefaultTimeoutSeconds` + = 0) and is overridable per job (`Job.TimeoutSeconds`, 0 = inherit the global + default). - **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the current process. Log files on disk feed aggregate statistics via `SeedStats` only. See [ARCHITECTURE.md](ARCHITECTURE.md). diff --git a/src/app/format.go b/src/app/format.go index ef40c26..06e81c6 100644 --- a/src/app/format.go +++ b/src/app/format.go @@ -105,11 +105,14 @@ func DisplayOverlapPolicy(job domain.Job, globalPolicy domain.OverlapPolicy) str // DisplayTimeout formats a job's effective run timeout for the details panel. // When the job sets its own TimeoutSeconds it is shown as-is; when 0 (inherit), // the global default is shown with "(global default)" appended, mirroring -// DisplayOverlapPolicy. +// DisplayOverlapPolicy. A non-positive global default means no timeout at all. func DisplayTimeout(job domain.Job, globalDefault int) string { if job.TimeoutSeconds > 0 { return fmt.Sprintf("%d s", job.TimeoutSeconds) } + if globalDefault <= 0 { + return "no timeout (global default)" + } return fmt.Sprintf("%d s (global default)", globalDefault) } diff --git a/src/app/format_test.go b/src/app/format_test.go index c62a585..00c8751 100644 --- a/src/app/format_test.go +++ b/src/app/format_test.go @@ -173,4 +173,7 @@ func TestDisplayTimeout(t *testing.T) { if got, want := DisplayTimeout(inherit, 30), "30 s (global default)"; got != want { t.Errorf("inherited timeout = %q, want %q", got, want) } + if got, want := DisplayTimeout(inherit, 0), "no timeout (global default)"; got != want { + t.Errorf("inherited infinite timeout = %q, want %q", got, want) + } } diff --git a/src/app/operations.go b/src/app/operations.go index f2193df..56fed0b 100644 --- a/src/app/operations.go +++ b/src/app/operations.go @@ -393,8 +393,8 @@ func validateConfig(config domain.Config) error { if config.OverlapPolicy != domain.OverlapPolicySkip && config.OverlapPolicy != domain.OverlapPolicyQueue { return errors.New("overlap policy must be 'skip' or 'queue'") } - if config.DefaultTimeoutSeconds <= 0 { - return errors.New("default timeout must be a positive number of seconds") + if config.DefaultTimeoutSeconds < 0 { + return errors.New("default timeout must not be negative (0 means no timeout)") } // Empty Theme is accepted and normalized to the default on load, so older // configs (and hand-built ones) stay valid without an explicit theme. diff --git a/src/app/operations_test.go b/src/app/operations_test.go index eedb964..208c904 100644 --- a/src/app/operations_test.go +++ b/src/app/operations_test.go @@ -527,7 +527,7 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) { {"missing logs dir", func(c *domain.Config) { c.LogsDir = "" }}, {"non-positive max files", func(c *domain.Config) { c.MaxLogFiles = 0 }}, {"non-positive max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }}, - {"non-positive default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = 0 }}, + {"negative default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = -1 }}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/src/app/run.go b/src/app/run.go index e4cc504..036bc37 100644 --- a/src/app/run.go +++ b/src/app/run.go @@ -214,7 +214,8 @@ func (s *Service) effectiveOverlapPolicy(job *domain.Job) domain.OverlapPolicy { // job's own TimeoutSeconds when positive, otherwise the global // Config.DefaultTimeoutSeconds. A non-positive Job.TimeoutSeconds means "inherit // the global default", which is why normalizeJob leaves 0 rather than -// backfilling the configured value. The caller must hold mu. +// backfilling the configured value. A resolved duration of 0 means no timeout; +// runner.RunJob treats it as "run without a deadline". The caller must hold mu. func (s *Service) effectiveTimeout(job *domain.Job) time.Duration { secs := job.TimeoutSeconds if secs <= 0 { diff --git a/src/app/run_test.go b/src/app/run_test.go index e241c50..c0b7ecc 100644 --- a/src/app/run_test.go +++ b/src/app/run_test.go @@ -633,4 +633,9 @@ func TestEffectiveTimeout(t *testing.T) { if got, want := svc.effectiveTimeout(own), 5*time.Second; got != want { t.Errorf("per-job timeout = %s, want %s", got, want) } + + svc.store.Config.DefaultTimeoutSeconds = 0 + if got, want := svc.effectiveTimeout(inherit), time.Duration(0); got != want { + t.Errorf("inherited timeout with no global default = %s, want %s (no timeout)", got, want) + } } diff --git a/src/domain/config.go b/src/domain/config.go index 00b8c73..853e7c9 100644 --- a/src/domain/config.go +++ b/src/domain/config.go @@ -54,7 +54,8 @@ type Config struct { ExecutionMode ExecutionMode `json:"execution_mode,omitempty"` OverlapPolicy OverlapPolicy `json:"overlap_policy,omitempty"` // DefaultTimeoutSeconds is the run timeout applied to jobs that do not set - // their own Job.TimeoutSeconds. It carries the formerly hard-coded 30s guard. + // their own Job.TimeoutSeconds. 0 (the default) means no timeout: such jobs + // run to completion however long that takes. DefaultTimeoutSeconds int `json:"default_timeout_seconds,omitempty"` Paused bool `json:"paused,omitempty"` // Theme selects the visual appearance. Empty is treated as ThemeDefault so @@ -77,7 +78,7 @@ func DefaultConfig() Config { ExecutionMode: ExecutionModeParallel, OverlapPolicy: OverlapPolicySkip, Theme: ThemeDefault, - DefaultTimeoutSeconds: 30, + DefaultTimeoutSeconds: 0, } } diff --git a/src/domain/job.go b/src/domain/job.go index d09a279..220f149 100644 --- a/src/domain/job.go +++ b/src/domain/job.go @@ -18,5 +18,6 @@ type Job struct { // TimeoutSeconds bounds how long a run may take before it is killed. 0 means // "inherit the global Config.DefaultTimeoutSeconds", mirroring OverlapPolicy: // normalizeJobs must leave 0 untouched rather than backfilling the default. + // The inherited global default may itself be 0, meaning no timeout at all. TimeoutSeconds int `json:"timeout_seconds,omitempty"` } diff --git a/src/runner/runner.go b/src/runner/runner.go index fe79641..3709cd2 100644 --- a/src/runner/runner.go +++ b/src/runner/runner.go @@ -18,8 +18,17 @@ func RunJob(ctx context.Context, job *domain.Job, trigger string, logsDir string started := time.Now() // Commands can hang forever if a script waits for input or a child process // stalls. The effective timeout is resolved by the caller (per-job value or - // the global default), keeping the runner ignorant of the global config. - runCtx, cancel := context.WithTimeout(ctx, timeout) + // the global default), keeping the runner ignorant of the global config. A + // non-positive timeout means "no timeout": context.WithTimeout(ctx, 0) would + // expire immediately, so fall back to a plain cancelable context that only + // ever ends via ctx (e.g. app shutdown). + var runCtx context.Context + var cancel context.CancelFunc + if timeout > 0 { + runCtx, cancel = context.WithTimeout(ctx, timeout) + } else { + runCtx, cancel = context.WithCancel(ctx) + } defer cancel() var output string diff --git a/src/runner/runner_test.go b/src/runner/runner_test.go index e972fdc..2cc0323 100644 --- a/src/runner/runner_test.go +++ b/src/runner/runner_test.go @@ -376,6 +376,31 @@ func TestRunJobTimesOut(t *testing.T) { } } +func TestRunJobZeroTimeoutMeansNoTimeout(t *testing.T) { + command := "sh" + arguments := "-c\nsleep 0.2" + if runtime.GOOS == "windows" { + command = `C:\Windows\System32\cmd.exe` + arguments = "/C\nping -n 2 127.0.0.1 >NUL" + } + job := domain.Job{ + ID: 52, + Name: "No Timeout Test", + Command: command, + Arguments: arguments, + } + + // A non-positive timeout must not expire immediately (context.WithTimeout + // with a zero duration would); the job must run to completion. + record, err := RunJob(context.Background(), &job, "Manual", t.TempDir(), 0) + if err != nil { + t.Fatal(err) + } + if record.State != "OK" { + t.Fatalf("expected job with no timeout to complete OK, got state %q detail %q", record.State, record.Detail) + } +} + func TestRunJobStartOnlyIgnoresTimeout(t *testing.T) { command := "sh" arguments := "-c\nsleep 5" diff --git a/src/storage/store_test.go b/src/storage/store_test.go index a95f2a1..aba884a 100644 --- a/src/storage/store_test.go +++ b/src/storage/store_test.go @@ -168,8 +168,8 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) { if got.MaxLogAgeDays != 30 { t.Errorf("default MaxLogAgeDays = %d, want 30", got.MaxLogAgeDays) } - if got.DefaultTimeoutSeconds != 30 { - t.Errorf("default DefaultTimeoutSeconds = %d, want 30", got.DefaultTimeoutSeconds) + if got.DefaultTimeoutSeconds != 0 { + t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds) } if got.Theme != domain.ThemeDefault { t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault) diff --git a/src/ui/settings_view.go b/src/ui/settings_view.go index 2120026..f4ed623 100644 --- a/src/ui/settings_view.go +++ b/src/ui/settings_view.go @@ -86,6 +86,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { overlapPolicySelect.SetSelected(string(store.Config.OverlapPolicy)) overlapPolicySelect.OnChanged = func(string) { updateSaveState() } defaultTimeout := widget.NewEntry() + defaultTimeout.SetPlaceHolder("0 = no timeout") defaultTimeout.SetText(strconv.Itoa(store.Config.DefaultTimeoutSeconds)) defaultTimeout.OnChanged = func(string) { updateSaveState() } jobsDir := widget.NewEntry() @@ -132,8 +133,8 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { return } timeout, err := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text)) - if err != nil || timeout <= 0 { - settingsStatus.SetText("Default timeout must be a positive number") + if err != nil || timeout < 0 { + settingsStatus.SetText("Default timeout must not be negative (0 = no timeout)") return } // Build the new config from the form and hand it to the Service, which