Phase 11 of PROJECT_REVIEW_PLAN.md: the themed cleanup pass over every
low-severity finding still open (2.2-2.3, 3.4-3.6, 4.3-4.7, 6.4-6.7,
7.1-7.3, 8.2-8.3, 9.1-9.4, and the under-documented decisions in §10/§11).
Behavioral fixes:
- Reassign duplicate job IDs in a hand-edited jobs.json instead of letting
two jobs share one runtime, schedule entry, and SeedStats bucket.
- Disambiguate run-log file names that collide within the same second.
- Compute AvgDurationMS as DurationSumMS/TimedRunCount instead of an
incremental integer mean, so it always matches the seeded-from-logs
average instead of drifting from truncation error.
- Clean absolute paths in ResolveConfiguredPath so two spellings of the
same jobs file do not trigger a spurious adoption.
- Report InstallDesktopIcon failures through ErrorOccurred instead of
discarding them silently.
- Move settingsView's blocking AutostartStatus (PowerShell on Windows) off
the UI thread.
- Give notify-timing.tsv its own extension so CleanupLogs no longer
manages it as a run log.
- Replace the settingsView Save handler's second copy of validateConfig's
rules with a bare parse, letting the Service's own error surface.
Cleanups:
- Delete collectActivity, the dead yaml tags on RunRecord, and the
logArguments/LogArguments alias.
- Fold the two systemTrayRegistered/mainWindowHidden globals into one
trayState instance Run owns and threads through Settings and the
single-instance reveal path.
- Fix stale comments/docs: the frozen window-size restore claim, a
reference to a renamed recordRun, README's "Pause all" and notification
wording, the PowerShell quoting note for TESTS.md's coverage command,
and scripts/test.bat's UTF-8 checkmarks under a non-UTF-8 code page.
- Document the single-instance fallback's consequence and the
unauthenticated instance-channel port in STANDARDS.md; record the
config-shim retirement plan in ROADMAP.md.
3.5, 7.3, and 9.4 turned out to already be fixed by earlier phases; no
change needed for those three.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 7 of the whole-project review (findings 3.2 and 3.3).
Service.mu is the lock the Fyne main thread takes on every Jobs() and
Runtime() call, so anything blocking inside it makes a UI refresh wait on
the disk. Three things did:
- Every SaveJobs/SaveConfig was a marshal, fsync, and rename under mu.
Writes are now prepared under the lock (Store.PrepareSaveJobs /
PrepareSaveConfig snapshot the payload and target path) and run after
it is released. deferSaveLocked takes saveMu while mu is still held, so
writes still reach the file in the order their snapshots were taken and
an older snapshot can never land on top of a newer one.
- executeRun ran runner.CleanupLogs under mu after every run. It needs
only the values already snapshotted into runEnv, so it now runs after
the unlock — including when the job is gone, since the run still wrote
a log file that retention covers.
- adoptJobsLocked ran runner.SeedStats under mu, reached from
UpdateSettings on the UI thread. Seeding moved out into
applySeededStatsLocked; UpdateSettings now reads the new jobs file and
seeds its statistics before taking the lock, and re-checks the
"no jobs-file switch while running" guard once it has it.
SeedStats also opened every log file twice — once to find the job, again
to read the result. readLogSummary reads job_id, state, and duration in
one pass, so each log is opened once.
StartOnly runs were built with exec.CommandContext on the app's lifecycle
context. os/exec keeps a watcher goroutine alive until Wait returns or the
context is done, and StartOnly never calls Wait, so one goroutine leaked
per run and would then try to kill a process whose handle startJobOnly had
already released. The invocation now uses context.Background(), whose nil
Done channel means no watcher is started at all.
Regression tests: TestRunJobStartOnlyLeavesNoContextWatcher (fails with 5
leaked goroutines on the old code), TestConcurrentJobOperationsLeaveTheFileMatchingMemory,
and TestUpdateSettingsSeedsAdoptedJobsFromLogs. STANDARDS gains the
no-I/O-under-mu rule and the "a StartOnly process outlives GoSentry" entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implements items 4-5 of the whole-project review's suggested order
(docs/PROJECT_REVIEW_PLAN.md):
- Drop the three SaveJobs calls in the run lifecycle (startRunLocked,
executeRun, SetGlobalPause): none of them change a durable Job field,
everything they touch lives on JobRuntime, which is never persisted.
Retire TestStartRunLockedRollbackOnSaveFailure with the rollback it
guarded, since a run can no longer fail to start this way.
- Clear PendingRuns (the "queue" overlap policy's backlog) when a job is
disabled or the scheduler is globally paused, so resuming or
re-enabling a job no longer replays a deferred run left over from
before the pause/disable. Cap it at maxPendingRuns (10) so a job whose
runs take longer than its own interval stops accumulating an unbounded
backlog. Surface the queued count in the details pane via DisplayStats.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
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.
Add an optional per-job run timeout following the overlap_policy inherit
pattern: Job.TimeoutSeconds (0 = inherit) resolves against a new
Config.DefaultTimeoutSeconds (default 30s), replacing the hard-coded 30s
guard in runner.RunJob.
- domain/storage: new fields, default 30, load-time normalization
- runner: RunJob takes an explicit timeout; StartOnly stays untimed so it
keeps measuring launch latency only
- app: effectiveTimeout resolves under mu into runEnv, threaded to runJob;
seam signature and validation updated; DisplayTimeout helper
- ui: Timeout entry in the job dialog, Default timeout in Settings, and a
Timeout row in the details panel
- tests + docs (ARCHITECTURE, STANDARDS, ROADMAP, CHANGELOG) updated;
version bumped to 0.12.0
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace overlap Pending flag with PendingRuns counter, match seed stats
by job_id, align average duration with TimedRunCount, and tidy docs.
Co-authored-by: Cursor <cursoragent@cursor.com>
Snapshot store paths under lock before async runs, roll back failed
start/save state, emit UI events only after successful persistence,
surface log write failures, and sync stale YAML docs to JSON.
Co-authored-by: Cursor <cursoragent@cursor.com>
- Allow manual "Run now" while the scheduler is paused: pause now stops only
automatic scheduled runs (RunDue), not the user's explicit action. Drop the
paused guard in Service.RunNow and the UI pause dialog; update tests.
- Cap the details metadata caption width via a new captionValueLayout so a wider
window feeds extra space to the value column instead of the short caption.
- Reorganize the Settings tab into two columns (Application+Queue / Storage+About)
with Save spanning the full width; move the Autostart status onto its own line.
- Condense the Jobs list rows with compactVBoxLayout to fit more jobs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RunDue now resolves each job's effective overlap policy via
effectiveOverlapPolicy (job value when set, else Config.OverlapPolicy)
instead of reading the global policy once per tick. An empty
Job.OverlapPolicy inherits the global default, so normalizeJobs continues
to leave the field untouched rather than backfilling it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add RunCount, FailCount, LastDurationMS, AvgDurationMS, MaxDurationMS
to JobRuntime and fold each completed RunRecord into them via updateStats
called from executeRun.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rework RunDue/startRunLocked/executeRun so the ExecutionMode and
OverlapPolicy config knobs take effect:
- startRunLocked advances NextDue to the next occurrence instead of
zeroing it, keeping the schedule marching during an in-flight run.
- RunDue scans all due jobs: parallel starts every due, non-running job;
sequential defers a due job while any other job runs. When a job comes
due again mid-run, skip drops it and queue marks it Pending; either way
NextDue is advanced past the fired occurrence.
- executeRun re-runs a Pending job once the current run finishes.
- RunNow gains a sequential-mode guard refusing a manual run while
another job is running.
- Add anyRunningLocked and advanceNextDueLocked helpers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracts the four dispatch functions and their private helper
runningOutput into a dedicated file so operations.go stays focused on
CRUD and settings. No behaviour change; shared helpers (prependLog,
refreshNextRunLocked, etc.) remain in operations.go where other
operations already call them.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mix
mixeme