Compare commits
1 Commits
v1.0.3
...
e05adc1703
| Author | SHA1 | Date | |
|---|---|---|---|
| e05adc1703 |
+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