@
feat: make per-job timeout 0 mean "no timeout" instead of inherit A per-job timeout now has three distinct states: unset inherits the global default, an explicit 0 means no timeout and does not inherit, and a positive value is the per-job limit. Job.TimeoutSeconds became *int so unset and 0 stay distinguishable in jobs.json. Also fixes the global default, which could not persist a 0. loadOrCreateConfig normalized DefaultTimeoutSeconds <= 0 back to 30 on every read of an existing gosentry.json, so "no timeout" only held until the next restart. The field is now written unconditionally (no omitempty) and read back as-is. Existing jobs and configs are unaffected: a job with no timeout_seconds still inherits, and a saved global default of 30 stays 30. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> @
This commit is contained in:
+15
-4
@@ -131,10 +131,21 @@ in flight increments `JobRuntime.PendingRuns`. When the current run finishes,
|
||||
|
||||
### Per-job command timeout
|
||||
|
||||
`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 **0**, i.e. no timeout); a
|
||||
positive value overrides it for that job alone. `app.Service.effectiveTimeout`
|
||||
`domain.Job` carries a `TimeoutSeconds *int` field
|
||||
(`json:"timeout_seconds,omitempty"`), following the same inherit pattern as the
|
||||
overlap policy. It is a **pointer** because the setting has three states that
|
||||
must stay distinguishable on disk:
|
||||
|
||||
| `Job.TimeoutSeconds` | jobs.json | Meaning |
|
||||
| --- | --- | --- |
|
||||
| `nil` | field absent | inherit `Config.DefaultTimeoutSeconds` |
|
||||
| `0` | `"timeout_seconds": 0` | no timeout, does **not** inherit |
|
||||
| `> 0` | `"timeout_seconds": 45` | per-job limit in seconds |
|
||||
|
||||
The global `Config.DefaultTimeoutSeconds` (default **0**, i.e. no timeout) is
|
||||
written unconditionally — no `omitempty` — for the same reason: `0` there is a
|
||||
deliberate choice, not a missing value, and `storage.loadOrCreateConfig` must not
|
||||
normalize it away. `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
|
||||
|
||||
@@ -2,6 +2,24 @@
|
||||
|
||||
All notable GoSentry changes are recorded in this file.
|
||||
|
||||
## Unreleased
|
||||
|
||||
**Timeouts: 0 now means "no timeout" at both levels.**
|
||||
|
||||
- The global **Default timeout** in Settings now defaults to `0`, meaning jobs
|
||||
run to completion with no deadline instead of being killed after 30s.
|
||||
- A per-job timeout of `0` now also means "no timeout" and no longer inherits
|
||||
the global default. Leaving the job's timeout **empty** is what inherits.
|
||||
`Job.TimeoutSeconds` became `*int` so the three states — unset, explicit 0,
|
||||
and a positive limit — stay distinguishable in `jobs.json`.
|
||||
- Fixed: a global default of `0` did not survive a restart. `gosentry.json` was
|
||||
loaded with `0` treated as a missing value and silently reset to 30s, so the
|
||||
setting only held for the current session. `default_timeout_seconds` is now
|
||||
written unconditionally and read back as-is.
|
||||
|
||||
Existing jobs and configs are unaffected: a job with no `timeout_seconds` still
|
||||
inherits, and a saved global default of 30 stays 30.
|
||||
|
||||
## 0.13.0 - 2026-07-26
|
||||
|
||||
**Branded GoSentry color theme; Cancel/Defaults buttons in Settings.**
|
||||
|
||||
+3
-2
@@ -18,8 +18,9 @@ in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
|
||||
- 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 no timeout globally (`Config.DefaultTimeoutSeconds`
|
||||
= 0) and is overridable per job (`Job.TimeoutSeconds`, 0 = inherit the global
|
||||
default).
|
||||
= 0) and is overridable per job (`Job.TimeoutSeconds *int`: unset = inherit the
|
||||
global default, 0 = no timeout, positive = seconds). Neither zero may be
|
||||
normalized away on load — 0 is a value, not a missing field.
|
||||
- **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).
|
||||
|
||||
+9
-5
@@ -103,12 +103,16 @@ 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. A non-positive global default means no timeout at all.
|
||||
// When the job sets its own TimeoutSeconds it is shown as-is, with an explicit 0
|
||||
// rendered as "no timeout"; when unset (nil), the global default is shown with
|
||||
// "(global default)" appended, mirroring 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 job.TimeoutSeconds != nil {
|
||||
if *job.TimeoutSeconds <= 0 {
|
||||
return "no timeout"
|
||||
}
|
||||
return fmt.Sprintf("%d s", *job.TimeoutSeconds)
|
||||
}
|
||||
if globalDefault <= 0 {
|
||||
return "no timeout (global default)"
|
||||
|
||||
@@ -165,11 +165,15 @@ func TestDisplayOverlapPolicy(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDisplayTimeout(t *testing.T) {
|
||||
own := domain.Job{TimeoutSeconds: 45}
|
||||
own := domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(45)}
|
||||
if got, want := DisplayTimeout(own, 30), "45 s"; got != want {
|
||||
t.Errorf("per-job timeout = %q, want %q", got, want)
|
||||
}
|
||||
inherit := domain.Job{TimeoutSeconds: 0}
|
||||
none := domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(0)}
|
||||
if got, want := DisplayTimeout(none, 30), "no timeout"; got != want {
|
||||
t.Errorf("explicit per-job zero timeout = %q, want %q", got, want)
|
||||
}
|
||||
inherit := domain.Job{TimeoutSeconds: nil}
|
||||
if got, want := DisplayTimeout(inherit, 30), "30 s (global default)"; got != want {
|
||||
t.Errorf("inherited timeout = %q, want %q", got, want)
|
||||
}
|
||||
|
||||
@@ -367,8 +367,8 @@ func validateJob(job domain.Job) error {
|
||||
if policy != "" && policy != string(domain.OverlapPolicySkip) && policy != string(domain.OverlapPolicyQueue) {
|
||||
return errors.New("overlap policy must be 'skip', 'queue', or empty")
|
||||
}
|
||||
if job.TimeoutSeconds < 0 {
|
||||
return errors.New("timeout must be zero (inherit) or a positive number of seconds")
|
||||
if job.TimeoutSeconds != nil && *job.TimeoutSeconds < 0 {
|
||||
return errors.New("timeout must be zero (no timeout) or a positive number of seconds, or unset to inherit the global default")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -103,9 +103,13 @@ func TestCreateJobValidates(t *testing.T) {
|
||||
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", OverlapPolicy: "invalid"}); err == nil {
|
||||
t.Error("expected error for invalid overlap policy")
|
||||
}
|
||||
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: -1}); err == nil {
|
||||
if _, err := svc.CreateJob(domain.Job{Name: "A", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: domain.TimeoutSecondsPtr(-1)}); err == nil {
|
||||
t.Error("expected error for negative per-job timeout")
|
||||
}
|
||||
// An explicit 0 is a valid choice ("no timeout"), not a rejected one.
|
||||
if _, err := svc.CreateJob(domain.Job{Name: "Zero", Schedule: "@every 1m", Command: "echo", TimeoutSeconds: domain.TimeoutSecondsPtr(0)}); err != nil {
|
||||
t.Errorf("explicit zero per-job timeout should be accepted: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateJobKeepsRuntimeAndReflectsDisable(t *testing.T) {
|
||||
|
||||
+7
-6
@@ -211,15 +211,16 @@ func (s *Service) effectiveOverlapPolicy(job *domain.Job) domain.OverlapPolicy {
|
||||
}
|
||||
|
||||
// effectiveTimeout resolves the run timeout that actually governs a job: the
|
||||
// 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
|
||||
// job's own TimeoutSeconds whenever it is set — including an explicit 0, which
|
||||
// means "no timeout" and deliberately does not inherit — otherwise the global
|
||||
// Config.DefaultTimeoutSeconds. A nil Job.TimeoutSeconds means "inherit the
|
||||
// global default", which is why normalizeJob leaves it nil rather than
|
||||
// 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 {
|
||||
secs = s.store.Config.DefaultTimeoutSeconds
|
||||
secs := s.store.Config.DefaultTimeoutSeconds
|
||||
if job.TimeoutSeconds != nil {
|
||||
secs = *job.TimeoutSeconds
|
||||
}
|
||||
return time.Duration(secs) * time.Second
|
||||
}
|
||||
|
||||
+12
-5
@@ -617,23 +617,30 @@ func TestRunDueQueueDrainSkippedWhenPaused(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestEffectiveTimeout verifies the inherit-or-override resolution: a zero
|
||||
// Job.TimeoutSeconds falls back to the global default, while a positive value
|
||||
// overrides it.
|
||||
// TestEffectiveTimeout verifies the three-state resolution: an unset (nil)
|
||||
// Job.TimeoutSeconds falls back to the global default, a positive value
|
||||
// overrides it, and an explicit 0 means "no timeout" without inheriting.
|
||||
func TestEffectiveTimeout(t *testing.T) {
|
||||
svc := newTempService(t, nil)
|
||||
svc.store.Config.DefaultTimeoutSeconds = 30
|
||||
|
||||
inherit := &domain.Job{TimeoutSeconds: 0}
|
||||
inherit := &domain.Job{TimeoutSeconds: nil}
|
||||
if got, want := svc.effectiveTimeout(inherit), 30*time.Second; got != want {
|
||||
t.Errorf("inherited timeout = %s, want %s", got, want)
|
||||
}
|
||||
|
||||
own := &domain.Job{TimeoutSeconds: 5}
|
||||
own := &domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(5)}
|
||||
if got, want := svc.effectiveTimeout(own), 5*time.Second; got != want {
|
||||
t.Errorf("per-job timeout = %s, want %s", got, want)
|
||||
}
|
||||
|
||||
// An explicit per-job 0 must beat a positive global default rather than be
|
||||
// mistaken for "unset".
|
||||
none := &domain.Job{TimeoutSeconds: domain.TimeoutSecondsPtr(0)}
|
||||
if got, want := svc.effectiveTimeout(none), time.Duration(0); got != want {
|
||||
t.Errorf("explicit per-job zero timeout = %s, want %s (no timeout)", 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)
|
||||
|
||||
@@ -53,10 +53,11 @@ type Config struct {
|
||||
NotifyOnFailure bool `json:"notify_on_failure,omitempty"`
|
||||
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. 0 (the default) means no timeout: such jobs
|
||||
// run to completion however long that takes.
|
||||
DefaultTimeoutSeconds int `json:"default_timeout_seconds,omitempty"`
|
||||
// DefaultTimeoutSeconds is the run timeout applied to jobs that leave their
|
||||
// own Job.TimeoutSeconds unset. 0 (the default) means no timeout: such jobs
|
||||
// run to completion however long that takes. It is written even when 0 —
|
||||
// omitempty would hide a deliberate choice from the hand-editable config.
|
||||
DefaultTimeoutSeconds int `json:"default_timeout_seconds"`
|
||||
Paused bool `json:"paused,omitempty"`
|
||||
// Theme selects the visual appearance. Empty is treated as ThemeDefault so
|
||||
// configs written before this field existed keep the original look.
|
||||
|
||||
+15
-5
@@ -15,9 +15,19 @@ type Job struct {
|
||||
StartOnly bool `json:"start_only,omitempty"`
|
||||
Enabled bool `json:"enabled"`
|
||||
OverlapPolicy string `json:"overlap_policy,omitempty"`
|
||||
// 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"`
|
||||
// TimeoutSeconds bounds how long a run may take before it is killed. It is a
|
||||
// pointer so the three states stay distinguishable on disk: absent (nil)
|
||||
// means "inherit the global Config.DefaultTimeoutSeconds", mirroring
|
||||
// OverlapPolicy's empty string; an explicit 0 means "no timeout" and does
|
||||
// not inherit; a positive value is the per-job limit in seconds. The
|
||||
// inherited global default may itself be 0, also meaning no timeout.
|
||||
// normalizeJobs must leave nil untouched rather than backfilling a value.
|
||||
TimeoutSeconds *int `json:"timeout_seconds,omitempty"`
|
||||
}
|
||||
|
||||
// TimeoutSecondsPtr returns a pointer suitable for Job.TimeoutSeconds. It exists
|
||||
// because nil (inherit) and an explicit 0 (no timeout) are different states, so
|
||||
// callers cannot just assign an int.
|
||||
func TimeoutSecondsPtr(seconds int) *int {
|
||||
return &seconds
|
||||
}
|
||||
|
||||
@@ -102,9 +102,10 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) {
|
||||
if config.OverlapPolicy == "" {
|
||||
config.OverlapPolicy = domain.OverlapPolicySkip
|
||||
}
|
||||
if config.DefaultTimeoutSeconds <= 0 {
|
||||
config.DefaultTimeoutSeconds = 30
|
||||
}
|
||||
// DefaultTimeoutSeconds is deliberately not normalized: 0 is a meaningful
|
||||
// value ("no timeout"), not a missing one, so backfilling it here would make
|
||||
// the setting impossible to persist. Negative values are rejected by
|
||||
// app.validateConfig before they can be saved.
|
||||
if config.Theme == "" {
|
||||
config.Theme = domain.ThemeDefault
|
||||
}
|
||||
|
||||
@@ -180,6 +180,63 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout"
|
||||
// setting against being normalized away when an existing gosentry.json is read
|
||||
// back. Loading must not treat 0 as a missing value.
|
||||
func TestLoadOrCreateConfigKeepsZeroTimeoutOnReload(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
paths := Paths{
|
||||
AppDir: dir,
|
||||
ConfigPath: filepath.Join(dir, ConfigFileName),
|
||||
}
|
||||
|
||||
// First call writes the defaults (DefaultTimeoutSeconds = 0) to disk.
|
||||
if _, err := loadOrCreateConfig(paths); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Second call takes the "file exists" branch, where normalization runs.
|
||||
reloaded, err := loadOrCreateConfig(paths)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if reloaded.DefaultTimeoutSeconds != 0 {
|
||||
t.Errorf("reloaded DefaultTimeoutSeconds = %d, want 0 (no timeout)", reloaded.DefaultTimeoutSeconds)
|
||||
}
|
||||
}
|
||||
|
||||
// TestJobTimeoutRoundTripsThreeStates pins the on-disk encoding that keeps
|
||||
// "inherit" and "no timeout" distinguishable: nil is omitted entirely, while an
|
||||
// explicit 0 is written and read back as a set value.
|
||||
func TestJobTimeoutRoundTripsThreeStates(t *testing.T) {
|
||||
jobs := []domain.Job{
|
||||
{ID: 1, Name: "Inherit", TimeoutSeconds: nil},
|
||||
{ID: 2, Name: "No timeout", TimeoutSeconds: domain.TimeoutSecondsPtr(0)},
|
||||
{ID: 3, Name: "Own", TimeoutSeconds: domain.TimeoutSecondsPtr(45)},
|
||||
}
|
||||
|
||||
data, err := json.Marshal(domain.JobsFile{Jobs: jobs})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if want := `"timeout_seconds":0`; !strings.Contains(string(data), want) {
|
||||
t.Fatalf("explicit zero timeout should be written as %s:\n%s", want, data)
|
||||
}
|
||||
|
||||
var got domain.JobsFile
|
||||
if err := json.Unmarshal(data, &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Jobs[0].TimeoutSeconds != nil {
|
||||
t.Errorf("unset timeout should stay nil, got %d", *got.Jobs[0].TimeoutSeconds)
|
||||
}
|
||||
if got.Jobs[1].TimeoutSeconds == nil || *got.Jobs[1].TimeoutSeconds != 0 {
|
||||
t.Errorf("explicit zero timeout should survive the round trip, got %v", got.Jobs[1].TimeoutSeconds)
|
||||
}
|
||||
if got.Jobs[2].TimeoutSeconds == nil || *got.Jobs[2].TimeoutSeconds != 45 {
|
||||
t.Errorf("per-job timeout should survive the round trip, got %v", got.Jobs[2].TimeoutSeconds)
|
||||
}
|
||||
}
|
||||
|
||||
func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) {
|
||||
// Job carries only durable configuration; runtime state lives in
|
||||
// domain.JobRuntime and is never marshalled. This guards against a future
|
||||
|
||||
@@ -55,9 +55,9 @@ func showJobDialog(w fyne.Window, title string, current job, onSave func(job)) {
|
||||
}
|
||||
overlapSelect.SetSelected(overlapSelected)
|
||||
timeoutEntry := widget.NewEntry()
|
||||
timeoutEntry.SetPlaceHolder("Empty = use global default")
|
||||
if current.TimeoutSeconds > 0 {
|
||||
timeoutEntry.SetText(strconv.Itoa(current.TimeoutSeconds))
|
||||
timeoutEntry.SetPlaceHolder("Empty = global default, 0 = no timeout")
|
||||
if current.TimeoutSeconds != nil {
|
||||
timeoutEntry.SetText(strconv.Itoa(*current.TimeoutSeconds))
|
||||
}
|
||||
|
||||
form := dialog.NewForm(
|
||||
@@ -89,16 +89,17 @@ func showJobDialog(w fyne.Window, title string, current job, onSave func(job)) {
|
||||
dialog.ShowError(fmt.Errorf("invalid schedule: %w", err), w)
|
||||
return
|
||||
}
|
||||
// An empty timeout inherits the global default (0); any entry must be a
|
||||
// An empty timeout inherits the global default (nil); an explicit 0
|
||||
// means "no timeout" and does not inherit; anything else must be a
|
||||
// positive whole number of seconds.
|
||||
timeoutSeconds := 0
|
||||
var timeoutSeconds *int
|
||||
if trimmed := strings.TrimSpace(timeoutEntry.Text); trimmed != "" {
|
||||
parsed, err := strconv.Atoi(trimmed)
|
||||
if err != nil || parsed <= 0 {
|
||||
dialog.ShowError(fmt.Errorf("timeout must be a positive number of seconds, or empty to use the global default"), w)
|
||||
if err != nil || parsed < 0 {
|
||||
dialog.ShowError(fmt.Errorf("timeout must be 0 (no timeout) or a positive number of seconds, or empty to use the global default"), w)
|
||||
return
|
||||
}
|
||||
timeoutSeconds = parsed
|
||||
timeoutSeconds = domain.TimeoutSecondsPtr(parsed)
|
||||
}
|
||||
current.Name = strings.TrimSpace(name.Text)
|
||||
current.Folder = strings.TrimSpace(folderEntry.Text)
|
||||
|
||||
Reference in New Issue
Block a user