Compare commits

8 Commits

Author SHA1 Message Date
mix 6a03ea4a20 release: v1.0.3
Bump version, update CHANGELOG, and retake README screenshots
(Jobs, History, Settings) to match the current GUI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-07 04:14:10 +03:00
mix 4fb9bf6ff3 docs: reconcile the documentation with the code
An audit of every document against the source turned up drift that had
accumulated since the 1.0.2 passes. The screenshot paths README and
DEVELOPMENT still point at are deliberately left alone - the images move
again when they are retaken.

ARCHITECTURE: the jobs_view.go split is six files, not five, since
extracting jobs_view_state.go was never counted; the statistics table
lists TimedRunCount, which the AvgDurationMS formula already referenced;
the store edge of the diagram names methods that exist (LoadJobs and
LoadConfig never did); and startup says that Service.Start is called from
newMainView rather than from Run.

TESTS: three tests had no entry, the latter two being regression tests
for 1.0.2 fixes:

  TestLoadOrCreateConfigPreservesZeroRetentionLimits
  TestWriteJSONReplacesFileAtomically
  TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension

The deliberately-uncovered list now covers everything the profile
actually reports at 0%, so the next redundancy pass does not flag Config,
Paths, SaveJobs, or the isEvent markers as gaps. The coverage section
measures through -coverprofile and says outright that the per-package
percentages -coverpkg prints are not the total - 2.6/8.9/25.5/1.0/61.5
against a real 84.1%.

ROADMAP: the over-the-guideline table was re-measured (service.go is over
it too now, making six), with a note to re-measure rather than trust it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-07 03:59:48 +03:00
mix 9b7a0565ba Rename 2026-08-07 03:43:32 +03:00
mix da8fe10365 docs: retire the whole-project review plan, correct its CHANGELOG traces
Every item in docs/PROJECT_REVIEW_PLAN.md is now either landed or moved to
ROADMAP.md, so the working document retires the way TEST_REVIEW_PLAN.md did.
Nothing else referenced it.

Two 1.0.2 Internal entries were out of step with what shipped: the timing
diagnostic is written as notify-timing.tsv (kept out of CleanupLogs and off
the UI thread), not notify-timing.log, and the removal of the two per-run
no-op jobs.json rewrites - along with the run-start rollback and the start
error RunDue used to surface - was not recorded at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-07 03:42:29 +03:00
mix 18da021526 chore: land the remaining low-severity items from the whole-project review
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>
2026-08-07 03:35:10 +03:00
mix bd7ebde68e refactor: extract the Jobs view state, track selection by job ID
Phase 10 of the whole-project review (findings 5.1 and 5.2), folded into
the ROADMAP file-split item as that plan asks.

5.2 was a real defect. `selected` was an index into a snapshot of the jobs
slice, and every path that changed the slice patched it by hand. The one
path that could not — adopting a different jobs file, where the Service
replaces the whole list and the view only hears about it through the
refresh JobsLoaded triggers — left the details pane redrawing from an
index that belonged to the previous list, describing whichever job now sat
there (or clearing when the new list was shorter) while the list highlight
stayed put. The selection is now a job ID; rows are derived from it at
render time, and refresh ends by pointing the highlight at the selected
job, so the two can no longer disagree.

5.1: newJobsView was one 330-line constructor whose dozen closures shared
seven mutable locals. It is now a jobsView struct over a jobsViewState
that owns the snapshot, the folder filter, and the selection — the
invariant that used to be maintained by hand in five places lives in one
place — split across jobs_view.go (construction, refresh, layout),
jobs_view_state.go, jobs_view_list.go, and jobs_view_toolbar.go. The
folder-option rebuild that appeared verbatim in three handlers is one
method.

Behaviour that changed beyond the fix: switching the folder filter keeps
the current selection when the new filter still shows it, instead of
always jumping to the folder's first job.

Docs: ARCHITECTURE records the new file layout and the selection-by-ID
contract; ROADMAP drops jobs_view.go from the over-guideline table and
refreshes the other five numbers (finding 2.4); TESTS documents the new
state test file and the adoption regression test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-06 22:38:47 +03:00
mix ca2a8c8aa7 fix: accept zero retention limits, retire Store() for typed accessors
Phase 8 (PROJECT_REVIEW_PLAN.md 8.1): 0 in MaxLogFiles/MaxLogAgeDays now
means "keep everything" end to end. runner.CleanupLogs already treated
<= 0 as disabled; validateConfig, the Settings form, and
loadOrCreateConfig's backfill were the only things making that state
unreachable.

Phase 9 (1.1, rolling up 1.2, 1.3, 7.3): added Service.Config() and
Service.Paths(), copying under mu, and converted every UI site that read
Service state through the raw *storage.Store returned by Store() (now
removed). jobs_view's pause control is now driven by refreshView reading
svc.Config().Paused on every event instead of only mirroring its own tap
handler, which makes it an actual consumer of SchedulerStateChanged.
mainwindow's event listener is a real type switch, and events.go's doc
comment no longer claims a compiler exhaustiveness check Go doesn't have.
Unexported the redundant SetAutostart/AutostartStatus package functions
in platform/autostart now that only the Manager methods are used outside
the package.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-06 22:15:19 +03:00
mix 0c8442a8d1 perf: keep file I/O off Service.mu and untie StartOnly from the app context
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>
2026-08-06 21:44:51 +03:00
56 changed files with 2016 additions and 1540 deletions
+9 -5
View File
@@ -14,8 +14,11 @@ creating, grouping, pausing, running, and monitoring scheduled shell commands.
<table> <table>
<tr> <tr>
<td align="center"><img src="images/screenshot_jobs.PNG" alt="Jobs tab"><br><em>Jobs tab — job list with details panel and run statistics.</em></td> <td align="center"><img src="docs/screenshots/screenshot_jobs.PNG" alt="Jobs tab"><br><em>Jobs tab — job list with details panel and run statistics.</em></td>
<td align="center"><img src="images/screenshot_settings.PNG" alt="Settings tab"><br><em>Settings tab — application, queue, storage, and version info.</em></td> <td align="center"><img src="docs/screenshots/screenshot_history.PNG" alt="History tab"><br><em>History tab — past runs with trigger, state, and log file.</em></td>
</tr>
<tr>
<td align="center" colspan="2"><img src="docs/screenshots/screenshot_settings.PNG" alt="Settings tab"><br><em>Settings tab — application, queue, storage, and version info.</em></td>
</tr> </tr>
</table> </table>
@@ -181,7 +184,7 @@ Named descriptors are also accepted: `@hourly`, `@daily`, `@weekly`,
3. Set **Schedule**, **Command**, optional **Arguments**, **Folder**, and **Enabled**. 3. Set **Schedule**, **Command**, optional **Arguments**, **Folder**, and **Enabled**.
4. Use **Run now** for a one-off manual run without waiting for the schedule. 4. Use **Run now** for a one-off manual run without waiting for the schedule.
5. Use **Pause** on a single job to suspend it without deleting it. 5. Use **Pause** on a single job to suspend it without deleting it.
6. Use **Pause all** as a global stop switch for all scheduled runs. 6. Use **Disable auto** as a global stop switch for all scheduled runs.
7. Open **History** to see past runs, their trigger (`Manual`, `Schedule`, or `UI`), state, and log file. 7. Open **History** to see past runs, their trigger (`Manual`, `Schedule`, or `UI`), state, and log file.
8. Open **Settings** to change the storage paths, log cleanup limits, queue behavior, and notifications. 8. Open **Settings** to change the storage paths, log cleanup limits, queue behavior, and notifications.
@@ -241,8 +244,9 @@ sets one.
## Notifications ## Notifications
When **Notify on failure** is enabled in Settings, GoSentry sends a desktop When **Notify on failure** is enabled in Settings, GoSentry sends a desktop
notification whenever a scheduled or manual run exits with a non-zero exit code. notification whenever a scheduled or manual run ends in the `Failed` state —
The notification shows the job name and the exit code. a non-zero exit code, a timeout, or a process that failed to start.
The notification shows the job name and the failure detail.
## Autostart ## Autostart
+1 -1
View File
@@ -17,7 +17,7 @@ import (
// The hard constraint: Fyne's a.SetIcon and SetSystemTrayIcon each take ONE // The hard constraint: Fyne's a.SetIcon and SetSystemTrayIcon each take ONE
// image, which the OS then scales to every size it needs — titlebar (~16px), // image, which the OS then scales to every size it needs — titlebar (~16px),
// taskbar/dock (~32-48px), and tray. Neither source survives that scaling: // taskbar/dock (~32-48px), and tray. Neither source survives that scaling:
// downscaling the 1254px gosentry-icon-big.png to 16px is muddy, and upscaling // downscaling the 1254px gosentry-icon-large.png to 16px is muddy, and upscaling
// the 16px icon to 32px is blurry. The fix is to feed each surface a // the 16px icon to 32px is blurry. The fix is to feed each surface a
// size-appropriate source — which differs per platform because each platform // size-appropriate source — which differs per platform because each platform
// exposes different icon channels. // exposes different icon channels.
+40 -19
View File
@@ -41,7 +41,7 @@ flowchart LR
user -->|"edits jobs, settings, runs commands"| ui user -->|"edits jobs, settings, runs commands"| ui
ui -->|"CreateJob, UpdateJob, DeleteJob, RunNow, UpdateSettings, AutostartStatus, …"| svc ui -->|"CreateJob, UpdateJob, DeleteJob, RunNow, UpdateSettings, AutostartStatus, …"| svc
svc -->|"SaveJobs, SaveConfig, LoadJobs, LoadConfig"| store svc -->|"OpenStore, PrepareSaveJobs, PrepareSaveConfig, LoadJobsFile"| store
store -->|"read/write"| config store -->|"read/write"| config
store -->|"read/write"| jobs store -->|"read/write"| jobs
@@ -119,10 +119,13 @@ example window-maximized detection, which would need per-OS native calls).
## Main Flows ## Main Flows
1. Startup: 1. Startup:
`cmd/gosentry` calls `ui.Run`, which creates an `app.Service`, opens the `cmd/gosentry` calls `ui.Run`, which owns the process lifecycle: it calls
store, loads `gosentry.json` and `jobs.json`, subscribes the UI to service `app.Open()` to open the store, load `gosentry.json` and `jobs.json`, and
events, builds the main window, and calls `Service.Start` to begin the build the `app.Service`, then hands that Service to `newMainView`
scheduler loop. On every launch the service seeds per-job run-time statistics (`mainwindow.go`), which subscribes the UI to service events and calls
`Service.Start` to begin the scheduler loop before assembling the tabs.
`Run` shows the window and, on quit, calls `Service.Stop`.
On every launch the service seeds per-job run-time statistics
from existing log files so the details panel reflects accumulated history from existing log files so the details panel reflects accumulated history
immediately (see §Statistics below). immediately (see §Statistics below).
@@ -136,10 +139,13 @@ example window-maximized detection, which would need per-OS native calls).
`UpdateSettings` has one extra step: when the configured jobs file changes `UpdateSettings` has one extra step: when the configured jobs file changes
and a file already exists at the new path, that file is authoritative. The and a file already exists at the new path, that file is authoritative. The
Service loads it, calls `adoptJobsLocked` to rebuild the jobs slice, runtime Service loads it, calls `adoptJobsLocked` to rebuild the jobs slice, runtime
map, schedule cache, next-run times, and log-seeded statistics around it, and map, schedule cache, and next-run times around it, applies the statistics
emits `JobsLoaded` plus a broad `JobChanged`. A path with no file behind it seeded from the new logs directory, and emits `JobsLoaded` plus a broad
receives the current jobs instead. Adoption drops all runtime state, so it is `JobChanged`. A path with no file behind it receives the current jobs instead.
refused while a job is running. Adoption drops all runtime state, so it is refused while a job is running.
Reading the new file and seeding its statistics both happen before `mu` is
taken (the no-I/O-under-`mu` rule in [STANDARDS.md](STANDARDS.md)), so the
running-job check is re-evaluated under the lock before anything is replaced.
3. Scheduled run: 3. Scheduled run:
`scheduler.Scheduler` fires a tick every second. On each tick it calls `scheduler.Scheduler` fires a tick every second. On each tick it calls
@@ -163,8 +169,9 @@ example window-maximized detection, which would need per-OS native calls).
6. History update: 6. History update:
When a run goroutine completes, `Service` updates the job's runtime When a run goroutine completes, `Service` updates the job's runtime
(including the statistics aggregate), saves JSON, triggers log cleanup, and (including the statistics aggregate) under `mu`, then — after releasing it —
emits `RunRecorded`. The UI observer appends the record to the History tab. runs log cleanup and emits `RunRecorded`. Nothing is saved: a run changes only
`JobRuntime`, which is never persisted. The UI observer appends the record to the History tab.
History rows exist only for the current process session; restarting the app History rows exist only for the current process session; restarting the app
clears the table (aggregate stats in the details panel are still seeded from clears the table (aggregate stats in the details panel are still seeded from
log files). log files).
@@ -220,9 +227,9 @@ resolves the effective duration under `mu` and `startRunLocked` snapshots it int
resolved duration as an argument, so the runner stays ignorant of the global resolved duration as an argument, so the runner stays ignorant of the global
config: a positive duration applies the timeout via `context.WithTimeout` and config: a positive duration applies the timeout via `context.WithTimeout` and
reports `Timed out after <timeout>` on expiry; a non-positive duration runs reports `Timed out after <timeout>` on expiry; a non-positive duration runs
without a deadline, bounded only by `ctx` (app shutdown). `StartOnly` jobs run on without a deadline, bounded only by `ctx` (app shutdown). `StartOnly` jobs are
the untimed context and so measure launch latency only, unaffected by the run built on `context.Background()` instead — neither the timeout nor app shutdown
timeout. applies to them — and so measure launch latency only.
### Run-time statistics ### Run-time statistics
@@ -233,8 +240,10 @@ timeout.
| `RunCount` | total runs recorded | | `RunCount` | total runs recorded |
| `FailCount` | runs that exited non-zero | | `FailCount` | runs that exited non-zero |
| `LastDurationMS` | wall-clock time of the most recent run (launch latency for `StartOnly`) | | `LastDurationMS` | wall-clock time of the most recent run (launch latency for `StartOnly`) |
| `AvgDurationMS` | mean over all runs with a recorded duration | | `AvgDurationMS` | mean over all runs with a recorded duration, computed as `DurationSumMS / TimedRunCount` on every update rather than folded incrementally, so it never disagrees with the exact sum/count average `runner.aggregateLogStats` computes when seeding from logs |
| `MaxDurationMS` | longest recorded run | | `MaxDurationMS` | longest recorded run |
| `TimedRunCount` | runs that carried a duration, and so contributed to the aggregates above; a legacy log without a `duration` header counts toward `RunCount` but not this |
| `DurationSumMS` | running total of every timed run's duration; the source `AvgDurationMS` is divided from |
`runner.RunJob` measures the wall-clock start→finish and sets `DurationMS` on `runner.RunJob` measures the wall-clock start→finish and sets `DurationMS` on
the returned `RunRecord`. `runner/logfile.go` writes a `duration` line into the the returned `RunRecord`. `runner/logfile.go` writes a `duration` line into the
@@ -266,16 +275,28 @@ the moment the window opens.
### `jobs_view.go` file structure ### `jobs_view.go` file structure
The size guideline for a file in this project is ~250 lines. The size guideline for a file in this project is ~250 lines.
`src/ui/jobs_view.go` is split across three files along these seams; the view `src/ui/jobs_view.go` is split across six files along these seams:
file itself has grown back over the guideline since — see the split item in
[ROADMAP.md](ROADMAP.md), which tracks every file currently over it:
| File | Contents | | File | Contents |
|------|----------| |------|----------|
| `jobs_view.go` | `newJobsView` — list, toolbar, button wiring, and layout | | `jobs_view.go` | `jobsView` struct — construction, `refresh`, `updateDetails`, the pause control, and layout assembly |
| `jobs_view_state.go` | `jobsViewState` — the jobs/runtime snapshot, the folder filter, and the selection |
| `jobs_view_list.go` | The sidebar list: row template, row rendering, row mode, and the compact/detailed toggle |
| `jobs_view_toolbar.go` | The per-job button row — new, edit, run, pause, delete |
| `jobs_view_details.go` | `detailsPanel` struct — widget creation, `update`, `clear`, `container` | | `jobs_view_details.go` | `detailsPanel` struct — widget creation, `update`, `clear`, `container` |
| `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs`, `nextJobListView`, `viewToggleText` | | `jobs_view_helpers.go` | Pure helpers — `filteredJobIndexes`, `folderOptions`, `filterValue`, `indexOfID`, `lastJobLogs`, `nextJobListView`, `viewToggleText` |
The widgets hold no job state of their own: they read `jobsViewState`, which is
the only thing that reads the Service. The **selection is a job ID, not a row
index.** Every path that changes the job list replaces the state's snapshot —
create, delete, and edit from this view's own handlers, adopting a different
jobs file from the Service, which the view only learns about through the refresh
`JobsLoaded` triggers. An index that outlives its snapshot points at whichever
job now sits there, so the details pane would describe one job while the list
highlighted another. Rows are derived from the ID at render time
(`selectedIndex`, `displayRow`), and `jobsView.refresh` ends by pointing the
list's highlight at the selected job.
### `settings_view.go` file structure ### `settings_view.go` file structure
`src/ui/settings_view.go` is split across three files the same way, once its `src/ui/settings_view.go` is split across three files the same way, once its
+147 -27
View File
@@ -2,6 +2,150 @@
All notable GoSentry changes are recorded in this file. All notable GoSentry changes are recorded in this file.
## 1.0.3 - 2026-08-07
**The findings of a whole-project review: durable JSON and log writes, bounded
History and overlap queues, and a Jobs selection that follows the job.**
**Application:**
- Fixed a Windows quoting bug where a job whose **Command** field held a whole
command line (a `.bat`/`.cmd` wrapper followed by an argument that itself
ended in `.exe`) had its entire command line mistaken for the program path,
so the run failed with an unmappable shell error. The program path is now
found by the earliest file-extension match at a word boundary, not the first
extension in list order.
- `gosentry.json` and `jobs.json` (and run log files) are now written
atomically — to a temp file, then renamed into place — so a crash or power
loss mid-write can no longer leave a truncated or empty file. `Service.Stop()`
is now called when the app quits, which also makes the run context
cancellation reach in-flight runs on shutdown.
- Fixed the "queue" overlap policy's backlog (`PendingRuns`): it no longer
survives a global pause or a job being disabled, so resuming or re-enabling a
job can no longer replay a deferred run left over from before the pause/
disable. It is also capped at 10 queued occurrences, so a job whose runs take
longer than its own interval no longer accumulates an unbounded backlog that
then runs back-to-back indefinitely. The job details pane now shows the
queued-run count (", N queued") whenever it is non-zero.
- **Start-only jobs are no longer tied to the application's lifetime.** A job
with *Start only* checked is launched on an uncancelable context, so quitting
GoSentry (or a run context being cancelled) can no longer try to kill a
process it deliberately stopped waiting for. This also removes a goroutine
that leaked on every start-only run and lived until the app exited.
- The History tab no longer grows without bound: it keeps the newest 1000
records and drops the oldest, the way a job's own activity list is capped.
Column widths are also folded in one record at a time instead of being
re-measured across every row on every event, so recording a run no longer
gets slower the longer the app has been running. Measured on 5000 accumulated
records, one History redraw went from **15.8 ms to 0.9 ms**; at the new cap
the width rescan alone accounted for 1.5 ms of every redraw.
- Two runs of the same job that start within the same second no longer share a
log file name. The later one gets a `-2`, `-3`, … suffix instead of silently
overwriting the earlier one's log — reachable with a fast manual re-run or a
sub-second queue drain.
- A hand-edited `jobs.json` in which two entries carry the same `id` no longer
leaves them sharing one runtime, one parsed schedule, and one statistics
bucket; the duplicate is reassigned a free ID on load, as an absent ID always
was.
- The **average run duration** shown in Statistics is now the exact sum divided
by the timed-run count rather than an incrementally folded integer mean. The
old form truncated on every run and the error compounded over a job's life,
so the live figure drifted away from the one rebuilt from log files after a
restart.
- On Linux, a failure to install the `.desktop` file or icon is now reported in
History instead of being discarded, so the visible symptom — a generic dock
icon — has an explanation.
**Jobs:**
- **The Jobs tab keeps its selection on the job, not on the row.** Selecting a
different jobs file in Settings replaces the whole job list; the details pane
then described whichever job happened to land on the previously selected row —
or went blank if the new list was shorter — while the highlight in the list
stayed where it was. The selection now follows the job itself, and the
highlight and the details pane always describe the same one.
- Switching the **Folder** filter now keeps the current selection when the new
filter still shows that job, instead of always jumping to the folder's first
job.
**Settings:**
- **Max log files and max log age days now accept 0, meaning "keep
everything."** Log cleanup already supported disabling either policy; the
Settings form and the Service validator rejected the value that would have
turned it on. A config that already set either to 0 is no longer silently
rewritten back to the 100/30 defaults on load.
- Opening the tab and saving no longer block the window while the autostart
status is read — on Windows that check shells out to PowerShell, and it now
runs off the UI thread.
- Two spellings of the same absolute **Jobs file** path (mixed separators, a
trailing separator) no longer read as a change of file, so saving no longer
triggers a spurious reload of the file already in use.
**Documentation:**
- Documentation audited against the code. `ARCHITECTURE.md` — the `jobs_view.go`
split is six files, not five (the state extraction was never counted), the
statistics table lists `TimedRunCount`, the store edge of the diagram names
the methods that exist, and startup says where `Service.Start` is actually
called. `TESTS.md` — three tests that had no entry are described
(`TestLoadOrCreateConfigPreservesZeroRetentionLimits`,
`TestWriteJSONReplacesFileAtomically`,
`TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension`), the
deliberately-uncovered list covers everything the profile reports at 0%, and
the coverage figure records how to read the total rather than the per-package
lines. `ROADMAP.md` — the over-the-guideline table was re-measured.
- README's scheduler wording caught up with the 0.11.2 rename of "Pause all" to
**Disable auto**, and its notification description matches what the app sends.
- `STANDARDS.md` records the rules the review settled: no file I/O under
`Service.mu`, the History and pending-run caps, the zero-retention meaning,
that a start-only process outlives GoSentry, the single-instance fallback's
consequence, and the unauthenticated instance-channel port.
- `docs/REVIEW.md` (the whole-project review agenda) and the working plan it
produced are retired now that every item is either landed here or recorded in
`ROADMAP.md`, the way the test review plan was in 1.0.1. `STANDARDS.md` is the
surviving reference.
- The screenshots moved to `docs/screenshots/`.
**Internal:**
- The failure-notification timing diagnostic added in 1.0.2 is now written to
`logs/notify-timing.tsv`. The `.tsv` extension keeps it out of `CleanupLogs`,
which manages only `.log` files, so it is neither deleted by age nor counted
against **Max log files**, and the append now runs off the UI thread.
- `jobs.json` is no longer rewritten twice per run. Starting and finishing a run
touch only `JobRuntime`, which is never persisted, so both saves re-serialised
identical bytes; `SetGlobalPause` did the same alongside its real `SaveConfig`.
Removing them also removes the run-start rollback path and the save failure it
reported, so `RunDue` no longer has a start error to surface at all.
- File I/O no longer happens while `Service.mu` is held — that is the lock the
UI thread takes on every job and runtime read, so a JSON write, the
post-run log cleanup, or the startup log scan used to make a UI refresh wait
on the disk. Saves are now prepared under the lock and written after it is
released, in preparation order, so `jobs.json` still ends up matching the
in-memory list. Seeding statistics from logs also opens each log file once
instead of twice.
- The Jobs tab was split into `jobs_view.go` (construction, refresh, layout),
`jobs_view_state.go` (the job/runtime snapshot, folder filter, and selection),
`jobs_view_list.go`, and `jobs_view_toolbar.go`, joining the existing
`jobs_view_details.go` and `jobs_view_helpers.go`. What used to be one
330-line constructor whose dozen closures shared seven mutable locals is now
widgets reading one named state object — which is what made the selection fix
above a change in one place instead of five.
- `Service.Store()` is replaced by typed `Service.Config()` and `Service.Paths()`
accessors that copy under the lock, so the UI no longer reaches into a shared
`*storage.Store`. The Jobs pause control is now driven by `refreshView`
reading `svc.Config().Paused` on every event, making it a real consumer of
`SchedulerStateChanged`, and the main window's event listener is a type switch.
- Dead code removed: `collectActivity`, the `yaml` tags on `RunRecord`, the
`logArguments`/`LogArguments` alias, the redundant package-level
`SetAutostart`/`AutostartStatus` functions, and the Settings Save handler's
second copy of the Service's validation rules. The
`systemTrayRegistered`/`mainWindowHidden` globals are one `trayState` value
that `Run` owns and threads through.
- `scripts/test.bat` no longer prints mojibake for its checkmarks under a
non-UTF-8 code page.
## 1.0.2 - 2026-08-05 ## 1.0.2 - 2026-08-05
**KeepRunningInTray is wired to runtime; Windows failure notifications can show **KeepRunningInTray is wired to runtime; Windows failure notifications can show
@@ -21,31 +165,6 @@ the app icon (experimental).**
- On Windows, failure toasts can show the app icon: after `NewWindow`, - On Windows, failure toasts can show the app icon: after `NewWindow`,
`AppMetadata.Icon` is registered so Fyne picks up artwork without calling `AppMetadata.Icon` is registered so Fyne picks up artwork without calling
`SetIcon`, which would override the PE multi-size window/taskbar icon. `SetIcon`, which would override the PE multi-size window/taskbar icon.
- Fixed a Windows quoting bug where a job whose **Command** field held a whole
command line (a `.bat`/`.cmd` wrapper followed by an argument that itself
ended in `.exe`) had its entire command line mistaken for the program path,
so the run failed with an unmappable shell error. The program path is now
found by the earliest file-extension match at a word boundary, not the first
extension in list order.
- `gosentry.json` and `jobs.json` (and run log files) are now written
atomically — to a temp file, then renamed into place — so a crash or power
loss mid-write can no longer leave a truncated or empty file. `Service.Stop()`
is now called when the app quits, which also makes the run context
cancellation reach in-flight runs on shutdown.
- Fixed the "queue" overlap policy's backlog (`PendingRuns`): it no longer
survives a global pause or a job being disabled, so resuming or re-enabling a
job can no longer replay a deferred run left over from before the pause/
disable. It is also capped at 10 queued occurrences, so a job whose runs take
longer than its own interval no longer accumulates an unbounded backlog that
then runs back-to-back indefinitely. The job details pane now shows the
queued-run count (", N queued") whenever it is non-zero.
- The History tab no longer grows without bound: it keeps the newest 1000
records and drops the oldest, the way a job's own activity list is capped.
Column widths are also folded in one record at a time instead of being
re-measured across every row on every event, so recording a run no longer
gets slower the longer the app has been running. Measured on 5000 accumulated
records, one History redraw went from **15.8 ms to 0.9 ms**; at the new cap
the width rescan alone accounted for 1.5 ms of every redraw.
**Jobs:** **Jobs:**
@@ -62,8 +181,9 @@ the app icon (experimental).**
**Internal:** **Internal:**
- App-side failure-notification timing is appended to `logs/notify-timing.log` - App-side failure-notification timing is appended to `logs/notify-timing.log`
for diagnosing toast delay (OS latency excluded). `scripts/measure-windows-toast.ps1` for diagnosing toast delay (OS latency excluded).
measures the PowerShell baseline on Windows. `scripts/measure-windows-toast.ps1` measures the PowerShell baseline on
Windows.
## 1.0.1 - 2026-08-04 ## 1.0.1 - 2026-08-04
+3 -2
View File
@@ -280,8 +280,9 @@ Before tagging:
1. Bump `src/app/version.go`. The tag must match it exactly. 1. Bump `src/app/version.go`. The tag must match it exactly.
2. Add the version's [CHANGELOG.md](CHANGELOG.md) section. 2. Add the version's [CHANGELOG.md](CHANGELOG.md) section.
3. Retake the README screenshots (`images/screenshot_jobs.PNG`, 3. Retake the README screenshots (`docs/screenshots/screenshot_jobs.PNG`,
`images/screenshot_settings.PNG`) if the GUI changed its appearance. This is `docs/screenshots/screenshot_settings.PNG`,
`docs/screenshots/screenshot_history.PNG`) if the GUI changed its appearance. This is
easy to forget because nothing fails without it: `README.md` is packaged easy to forget because nothing fails without it: `README.md` is packaged
inside every release archive and is what the forge shows on the project page, inside every release archive and is what the forge shows on the project page,
so a stale shot advertises an application that no longer exists. Take them so a stale shot advertises an application that no longer exists. Take them
-793
View File
@@ -1,793 +0,0 @@
# Whole-project review — action plan
Working document for the findings of the 2026-08-05 whole-project review. It is
not part of the permanent doc set: delete it once every item below is either
done or moved to [ROADMAP.md](ROADMAP.md), the way `TEST_REVIEW_PLAN.md` was
retired.
The rules the findings are judged against live in [STANDARDS.md](STANDARDS.md)
and [ARCHITECTURE.md](ARCHITECTURE.md). Anything listed under "Intentional
behavior" in STANDARDS is not reported as a bug; where this review disagrees
with such an entry it says so explicitly as a **challenge**.
## Baseline the review started from
Measured on the 1.0.2 tree (`c8a4d31`), MSYS2 UCRT64 / CGO on:
- `go vet ./...` — clean.
- `go test -race ./...` — all packages pass. `src/ui` alone takes **229 s**;
everything else finishes in under 8 s.
- Engine coverage, merged profile over `domain`, `storage`, `runner`,
`scheduler`, `app`: **84.0%** (TESTS.md records 84.4% at the 2026-08-04
review). The 0.4 pp dip is *not* item 4.1 — it is new 1.0.2 code that arrived
untested: `storage.PeekKeepRunningInTray` sits at 0%. It is a startup entry
point like `OpenStore` and `ResolvePaths`, so if it is meant to stay
uncovered it belongs in TESTS.md's "Functions deliberately at 0%" list, which
currently does not name it.
- 171 test functions in the tree. TESTS.md names 171 as well, but the sets do
not match: two of the names it documents no longer exist, and two tests that
do exist are undocumented (items 4.1, 4.2).
- 77 Go files, ~10 500 lines including tests. Two direct dependencies.
Overall finding: **the project is in good health.** The engine layering
(`domain``storage`/`runner`/`scheduler``app``ui`) holds, the locking
contract on `Service.mu` is stated and obeyed, and the UI layout code — usually
the first thing to rot in a desktop app — is the strongest part of the codebase:
sizes are measured from the theme, the helpers are named, and the geometry is
pinned by tests that re-run under a scaled theme. The documentation set is
unusually complete and, with the exceptions in §4, accurate.
The findings below are therefore mostly about **the paths that only show up
after the app has been running for a while** (§3.1, §6.3), **durability of the
JSON files** (§6.2), and **one confirmed Windows quoting bug** (§6.1).
Severity follows the whole-project review convention: *medium* means it gets a
regression test with the fix.
---
## 1. Architecture and project structure
### 1.1 `Service.Store()` is the hole in "the Service is the sole owner" — medium
[service.go:170](../src/app/service.go) hands callers the raw `*storage.Store`.
Its own doc calls the surface transitional ("later phases narrow this"); the
phase never came. Eight UI sites read Service-owned state straight through it:
| Site | Reads |
|---|---|
| [jobs_view.go:60,61,64,66,79](../src/ui/jobs_view.go) | `Config.Paused`, `Config.JobListView`, `Config.OverlapPolicy`, `Config.DefaultTimeoutSeconds` |
| [settings_view.go:30](../src/ui/settings_view.go) | the whole `Config`, held as a live pointer for the session |
| [run.go:73,78](../src/ui/run.go) | `Config.KeepRunningInTray`, `Config.Theme` |
| [mainwindow.go:92](../src/ui/mainwindow.go) | `Paths.LogsDir`, from inside the notification path |
This contradicts ARCHITECTURE ("the UI reads it through typed events, never
through shared mutable state") and STANDARDS. It is not a live data race
**today**, but only because of an invariant nothing writes down and nothing
enforces: every writer of `store.Config` (`UpdateSettings`, `SetGlobalPause`,
`SetJobListView`) happens to be reached from the Fyne main thread, so the
unlocked UI reads are serialised with them by accident. One background writer —
say, a future auto-reload of `jobs.json`, or moving log cleanup off the UI
thread — turns all eight into races that `-race` will not catch, because no test
drives them concurrently.
Fix: give the Service typed accessors that copy under `mu` (`Config()`,
`LogsDir()`), convert the eight call sites, and either unexport `Store()` or
reduce it to what the tests actually need.
### 1.2 `SchedulerStateChanged` is emitted and never consumed — low
[events.go:38](../src/app/events.go) documents it as "The UI uses it to update
the pause/resume control and status text." No observer handles it: the single
listener in [mainwindow.go:67](../src/ui/mainwindow.go) type-asserts only
`RunRecorded`, `ErrorOccurred`, and `JobsLoaded`. The Jobs toolbar keeps its own
`schedulerPaused` copy and relabels the button inside its own tap handler
([jobs_view.go:271](../src/ui/jobs_view.go)).
It works because the tap handler is the only thing that can pause today. That is
exactly the coupling the event bus exists to remove. Either consume the event
and delete the local mirror, or delete the event and drop the claim.
### 1.3 The "exhaustive type-switch" the doc promises does not exist — low
The same comment block says the sealed `Event` interface means "a UI listener
can exhaustively type-switch over them and the compiler will flag a new event
type that a switch forgot to handle." Go has no exhaustiveness check on type
switches, and the one listener does not even use a switch — it uses three
independent assertions. The comment claims a safety property that is not there,
which is how 1.2 went unnoticed. Reword it to say what sealing actually buys
(observers cannot be handed an event type from outside the package).
---
## 2. Complexity against the size of the project
Nothing here is over-abstracted: the `Clock` interface, the `runJob` seam, and
the `autostart.Manager` interface each have a real test seam or a real second
implementation. The findings run the other way — code that is still there after
its reason left.
### 2.1 Two full `jobs.json` rewrites per run that cannot change the file — medium
[startRunLocked](../src/app/run.go) calls `s.store.SaveJobs(s.jobs)` on every run
start and [executeRun](../src/app/run.go) calls it again on every run finish.
Neither function assigns to a single `domain.Job` field: everything they touch
lives on `JobRuntime`, which is explicitly never persisted
([runtime.go:5](../src/domain/runtime.go)). Both calls therefore re-serialise
and rewrite the identical bytes. `SetGlobalPause`
([operations.go:182](../src/app/operations.go)) does the same — its durable
change is `Config.Paused`, saved separately by `SaveConfig`.
The cost is not only I/O. `startRunLocked` carries a five-line rollback block
and a regression test (`TestStartRunLockedRollbackOnSaveFailure`) guarding a
write that can never change the file's content, and the write happens under
`Service.mu` (see 3.2). Removing the three calls removes the I/O, the rollback,
and the failure mode at once.
Care needed: the review found no durable field written on these paths, but this
should be re-verified against the `domain.Job` definition when the change is
made, and `TestStartRunLockedRollbackOnSaveFailure` retired deliberately rather
than left failing.
### 2.2 `runner.logArguments` is an alias of `runner.LogArguments` — low
[invocation.go:68](../src/runner/invocation.go) —
`func logArguments(a string) string { return LogArguments(a) }`. A leftover from
exporting the function. Four call sites; inline them and delete it.
### 2.3 `collectActivity` always returns an empty slice at startup — low
[mainwindow.go:30-37](../src/ui/mainwindow.go) builds an `initialRuntimes` map
purely to feed [collectActivity](../src/ui/history_view.go), which merges
`JobRuntime.Logs` across jobs. History is session-only by design, so at
construction time every `Logs` slice is empty and the result is always `nil`.
The function's own comment says it is kept "for future history loading from log
metadata" — a feature that is not on the ROADMAP.
Either delete the twelve lines, or record the placeholder in STANDARDS so the
next reviewer does not re-report it. Its two unit tests are fine either way —
they test the merge, not the caller.
### 2.4 The ROADMAP size table is stale — info
[ROADMAP.md](ROADMAP.md) lists the files over the ~250-line guideline as of
1.0.0. Measured today:
| File | ROADMAP | Now |
|---|---|---|
| `src/app/operations.go` | 490 | 490 |
| `src/ui/jobs_view.go` | 355 | 361 |
| `src/ui/settings_view.go` | 277 | **304** |
| `src/storage/store.go` | 265 | **299** |
| `src/app/run.go` | 287 | 287 |
| `src/ui/history_view.go` | 282 | 282 |
Refresh the numbers when the split item is picked up; the trend is the point,
not the individual figures.
---
## 3. Code quality
### 3.1 History grows without bound, and every run pays for it — medium
This is the most consequential finding in the review, because it only appears in
the mode the app is designed to run in: left in the tray for days.
`events` in [mainwindow.go:73](../src/ui/mainwindow.go) is appended to on every
`RunRecorded` and never trimmed. Each entry is a full `domain.RunRecord`,
including `Output` — the complete captured stdout and stderr of the run. Then,
on every single event, `refresh()` runs:
- `resort()` — copies the whole slice and sorts it
([history_view.go:181](../src/ui/history_view.go));
- `setColumnWidths()``historyColumnWidths(rows)` — builds three
slices of length *n* and calls `fyne.MeasureText` once per non-empty value in
each of the Job, Detail, and Log columns
([history_view.go:104](../src/ui/history_view.go)).
So the per-run cost is O(*n* log *n*) sorting plus up to 3*n* text measurements
on the UI thread, with *n* growing forever. One job on `@every 10s` produces
~8 600 records a day. `JobRuntime.Logs` is capped at 50 by `maxJobLogs`; the
History slice — the one that actually accumulates — is not capped at all.
Fix in two parts: cap the History slice (a ring buffer, or a `maxHistoryRows`
mirroring `maxJobLogs`), and stop rescanning every row for column widths on
every event — widths only ever grow, so fold the new record into the current
maxima instead of recomputing from scratch.
This is the one finding worth a measurement before and after, since STANDARDS
already treats measured geometry as the standard of proof.
### 3.2 Blocking file I/O under `Service.mu` — medium
`Service.mu` is the lock the Fyne main thread takes on every `Jobs()` and
`Runtime()` call — that is, on every UI refresh. Three things do file I/O while
holding it:
- [executeRun](../src/app/run.go) calls `runner.CleanupLogs` — a directory scan
plus up to `MaxLogFiles` unlinks — under `mu`, after every run.
- Every `SaveJobs` / `SaveConfig` is a full JSON marshal and write under `mu`.
- [adoptJobsLocked](../src/app/service.go) calls `runner.SeedStats` under `mu`,
reached from `UpdateSettings` on the UI thread.
None of it needs the lock: cleanup takes only the values already snapshotted
into `runEnv`, and seeding only needs the job list. Move them outside the
critical section, or snapshot and run them after `mu.Unlock()` the way the event
emission already does.
`SeedStats` also opens every log file **twice** — once in `readLogJobID` and
again in `readLogHeader` ([seed.go:59,98](../src/runner/seed.go)) — and the
first pass is not bounded by `maxFiles`, so it touches every `.log` in the
directory. One pass returning `(jobID, state, duration)` halves the syscalls.
### 3.3 `StartOnly` leaks a goroutine per run and mis-owns the process — medium
[runner.go:39](../src/runner/runner.go) builds the fire-and-forget invocation
with `jobInvocation(ctx, …)`, which uses `exec.CommandContext`. After `Start()`,
os/exec spawns a watcher goroutine that blocks until either `Wait()` returns or
the context is done. `StartOnly` never calls `Wait` — that is the whole point —
so the goroutine lives until the app exits, one per StartOnly run, and then
calls `Kill` on a process whose handle `startJobOnly` already `Release`d.
The kill is harmless in practice (a released handle makes it fail), but the
leak is real and the ownership is the wrong shape: a job the runner explicitly
stops waiting for should not be tied to the app's lifecycle context at all. Use
`exec.Command` (or `context.Background()`) for the StartOnly branch and say in
STANDARDS whether a started process is expected to outlive GoSentry.
### 3.4 `InstallDesktopIcon` swallows its error — low
[platform.go:11](../src/app/platform.go) —
`if iconPath, err := desktop.InstallDesktopIntegration(…); err == nil { … }`.
The error is discarded with no dialog, no History event, and no log line. That
is the silent `return` STANDARDS forbids. On Linux the visible symptom is a
generic dock icon with no explanation. Emit `ErrorOccurred`.
### 3.5 `RunDue` keeps only the last start error — low
[run.go:92](../src/app/run.go) — `startErr = err; continue`. If two jobs fail to
start on the same tick, the user sees one message. Join them (`errors.Join`) or
emit one event per failure.
### 3.6 Settings re-implements `validateConfig` — low
[settings_view.go:136-158](../src/ui/settings_view.go) validates max log files,
max log age, jobs file, logs dir, and default timeout with its own messages,
before `UpdateSettings` validates the same five with different messages
([operations.go:456](../src/app/operations.go)). The UI genuinely needs the
`strconv` parse; it does not need a second copy of the rules. Parse in the UI,
validate in the Service, and show what the Service returns.
---
## 4. Documentation and comments
The doc set is accurate about design and rationale. What has drifted is the
inventory.
### 4.1 Two documented tests were silently deleted — medium
[TESTS.md](TESTS.md) lists `TestJobListViewIsCompact` and
`TestDefaultConfigUsesDetailedJobList` under `src/domain/config_test.go`.
Neither exists. Commit `5b0e6fe` ("Wire KeepRunningInTray to runtime …")
**rewrote** that file to hold `TestAutostartArguments` and
`TestResolveStartHidden` instead of appending them, and the two older tests went
with it.
**This was not the test-suite review's doing, and it was not a decision.** The
2026-08-04 review deleted exactly three tests — `TestCleanupLogsKeepsFilesWithinAgeLimit`,
`TestRunDueEmptyOverlapInheritsGlobal`, and `TestSameWindowsPathHandlesSpaces`
each after measuring byte-identical coverage against a survivor whose assertions
were a superset, and each recorded in `TEST_REVIEW_PLAN.md` and in the CHANGELOG.
Its deletion commit `2ef18e7` never opened `config_test.go`; only `29ce94c`
(which created the two tests) and `5b0e6fe` ever touched that file.
What settles it is what `5b0e6fe` did to the documentation: it **added** the two
new test rows to TESTS.md while **leaving the two old rows in place**, i.e. it
documented the file as holding all four. The doc moved in the opposite direction
from the code. A deliberate removal looks like `2ef18e7`, which took its three
rows out of TESTS.md in the same commit. Nothing in the commit message, the
1.0.2 CHANGELOG, or STANDARDS mentions the loss.
Accidental, however, does not mean both are worth having back. Only one was
pulling weight:
- **`TestJobListViewIsCompact` — restore it.** Its unique assertions are that
`""` and a differently-cased `"Compact"` both read as detailed. Neither holds
anywhere else now: `TestSetJobListViewNormalizesUnknownValue` (`app`) covers
only the unrecognised-value path through `SetJobListView`. The empty case is
live rather than theoretical — `loadOrCreateConfig` does **not** normalize
`job_list_view` the way it normalizes `theme`, so a config written before the
field existed reaches `IsCompact()` empty and depends on exactly this
behaviour. STANDARDS §Config file compatibility also requires it by name:
"Each of the three gets a test: the default in `storage`, the normalization
in `domain`, and a round-trip through the real config file in `app`." The
`domain` one is the one that disappeared, so a rule STANDARDS calls mandatory
is currently unenforced.
- **`TestDefaultConfigUsesDetailedJobList` — do not restore it; take its row
out of TESTS.md instead.** `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun`
(`storage`) already asserts `got.JobListView == domain.JobListViewDetailed`,
through the real load path, which makes it a strict superset — and STANDARDS
puts the default test in `storage`, not `domain`. Under TESTS.md principle 9
this is a legitimate deletion; it simply was never made deliberately.
Neither loss moved the number: `IsCompact` and `DefaultConfig` both measure
100% today, exercised through their callers. What was lost is an assertion, not
statement coverage — which is the exact case TESTS.md principle 9 exists to
name ("Identical coverage alone is *not* grounds for deletion").
The surviving test is recoverable verbatim from
`git show 5b0e6fe^:src/domain/config_test.go`.
### 4.2 `src/ui/notify_timing_test.go` is undocumented — low
`TestNotificationTimingFormatLine` and
`TestAppendNotificationTimingLogWritesHeaderAndRow` were added in 1.0.2 with no
TESTS.md entry. Add the file's table.
### 4.3 The window-size comment describes a feature that is frozen — low
[run.go:19](../src/ui/run.go): "later launches restore the last size from
preferences." Nothing ever writes `window.width` / `window.height` — ROADMAP
records the feature as deliberately frozen. The comment is wrong and the two
`prefs.FloatWithFallback` reads are dead code that make it look implemented.
See also 8.2.
### 4.4 README says "Pause all"; the button says "Disable auto" — low
[README.md](../README.md) step 6 under *Using The App*. The control is labelled
`Disable auto` / `Enable auto` ([jobs_view.go:261](../src/ui/jobs_view.go)).
### 4.5 A comment cites a function that no longer exists — low
[jobs_view_helpers.go:11](../src/ui/jobs_view_helpers.go) refers to
`app.Service.recordRun`. The function is `executeRun`.
### 4.6 README narrows when notifications fire — low
"…whenever a scheduled or manual run exits with a non-zero exit code." The
condition is `State == "Failed"`, which also covers timeouts and processes that
failed to start.
### 4.7 The coverage command in TESTS.md does not run on the documented shell — info
TESTS.md gives the `-coverpkg` invocation in bash form. In the PowerShell
environment DEVELOPMENT.md prescribes for Windows, PowerShell splits the
comma-separated package list and the command fails with
`directory not found`. It needs `--%` (or the whole flag quoted). Worth a note
next to the command, since it is the one measurement the doc asks reviewers to
reproduce.
---
## 5. Readability and maintainability
### 5.1 `newJobsView` is one 330-line constructor over shared mutable locals — medium
[jobs_view.go:30-361](../src/ui/jobs_view.go). Twelve closures share
`jobs`, `runtimes`, `selected`, `selectedFolder`, `filteredJobs`, `listView`,
and `schedulerPaused`, and several of them patch two or three of those in
sequence before calling `refreshView`. Understanding any one handler means
reading all of them, because the invariant "`selected` indexes `jobs`, and the
list's selection index indexes `filteredJobs`" is maintained by hand in five
places.
ROADMAP already tracks the split. This review adds the reason it matters beyond
line count: the state, not the length, is what makes it hard. Extracting a small
`jobsViewState` struct with `selectByID`, `applyFilter`, and `snapshot` methods
would shrink the file and make 5.2 impossible.
### 5.2 Selection is tracked by slice index, not by job ID — medium
`selected` is an index into a snapshot of the jobs slice. Every path that can
change the slice — create, delete, filter — patches it explicitly. The path that
replaces the whole list does not: adopting a different jobs file emits
`JobsLoaded` plus a broad `JobChanged`, the observer calls `refresh()`, and
`refreshView` calls `updateDetails(selected)` with an index from the *previous*
list. The details pane then describes whichever job now happens to sit at that
index, while the list's highlight is untouched.
Track the selection by `Job.ID` and resolve it to an index at render time.
### 5.3 `operations.go` mixes three jobs in one file — low
490 lines: the public mutating operations, the `…Locked` state helpers only they
call, and the pure validators/normalizers. ROADMAP already names this as the
clearest of the six splits; nothing to add except that it is still the worst
overage.
### 5.4 The nested `fyne.Do` has no explanation — low
[mainwindow.go:71 and 85](../src/ui/mainwindow.go) — the observer's body already
runs inside `fyne.Do`, and the failure-notification block opens a second one.
The nesting is deliberate (it defers the toast by one main-thread hop so
`UIQueuedAt` can measure that hop for `notify-timing.log`), but nothing says so,
and a reader's first instinct is to "simplify" it away and lose the
instrumentation. The same block also calls `appendNotificationTimingLog` — a
file open, stat, and write — on the UI thread.
Add the sentence that explains the nesting, and move the log append off the main
thread.
---
## 6. Logical errors
### 6.1 Windows shell quoting picks the wrong program path — medium (reproduced)
[quoteLeadingWindowsProgramPath](../src/runner/invocation_windows.go) walks the
extension list `.exe`, `.cmd`, `.bat`, `.com` **in list order** and takes the
first extension that appears anywhere in the string. It should take the
extension that appears *earliest*, and only at a token boundary. When the
program is a `.bat` or `.cmd` and any argument ends in `.exe`, the `.exe` in the
argument is found first and the entire command line is treated as the program
path.
Reproduced by running the function verbatim outside the build:
| Input (job `Command`) | Produced command line |
|---|---|
| `C:\My Tools\run.bat D:\in.txt` | `cmd.exe /S /C ""C:\My Tools\run.bat" D:\in.txt"` ✔ |
| `C:\My Tools\run.bat C:\Windows\System32\notepad.exe` | `cmd.exe /S /C ""C:\My Tools\run.bat C:\Windows\System32\notepad.exe""` ✘ |
| `C:\Program Files\App\deploy.cmd D:\stage\setup.exe` | `cmd.exe /S /C ""C:\Program Files\App\deploy.cmd D:\stage\setup.exe""` ✘ |
| `C:\dir.exexample\My Tool\run.bat` | `cmd.exe /S /C "C:\dir.exexample\My Tool\run.bat"` ✘ (never quoted) |
The two ✘ rows in the middle hand `cmd.exe` a single quoted token that is not a
file, so the run fails with a shell-level error the user cannot map back to
their job. The last row is the mirror image: a `.exe` substring inside a
directory name makes the check conclude the program path has no spaces, so a
path that *does* need quoting is left bare.
Reachable through normal use: it applies whenever the command does not resolve
as a direct executable path, which is what happens when the user types a whole
command line into the **Command** field — the shape the field's own placeholder
and the existing Joplin test fixture both demonstrate.
Fix: find the earliest extension match across all four extensions, and require
the character after it to be a space or end-of-string. Regression test with the
four rows above.
### 6.2 `gosentry.json` and `jobs.json` are written non-atomically — medium
[storage.writeJSON](../src/storage/store.go) is `os.WriteFile` — truncate, then
write. A crash, a power loss, or the process exiting during the write leaves a
truncated or empty file, and for `jobs.json` that is every job definition the
user has.
The exposure is larger than it looks because of 2.1: `SaveJobs` runs twice per
run, so the window is open constantly on a busy install. And `Service.Stop()` is
never called — `ui.Run` has no shutdown path, and the tray's Quit item goes
straight to `a.Quit()` ([tray.go:72](../src/ui/tray.go)) — so quitting while a
run is completing terminates the process mid-write with nothing to flush.
Fix: write to `<name>.tmp` in the same directory, `Sync`, then `os.Rename` over
the target. Rename is atomic within a volume on both supported platforms. The
same treatment is cheap for `runner/logfile.go`, though a torn log file costs
much less than a torn jobs file.
Worth pairing with a `Service.Stop()` call on shutdown, which also makes the
`ctx` cancellation the runner already implements actually reachable.
### 6.3 `PendingRuns` survives a pause and has no ceiling — medium
[executeRun](../src/app/run.go) drains the queue with
`rerun := runtime.PendingRuns > 0 && current.Enabled && !s.paused`. Nothing ever
*clears* the counter. Two consequences:
- **Pause leaks a run.** Pause the scheduler while a `queue`-policy job has a
backlog, and the counter stays set. `refreshNextRunLocked` parks the job at
"Scheduler paused" and the drain is skipped — correctly, and
`TestRunDueQueueDrainSkippedWhenPaused` pins that. But after the user resumes,
the stale counter is still there, and the next completed run of that job fires
a deferred run that corresponds to an occurrence from before the pause.
Disabling a job has the same shape: `SetEnabled(false)` does not clear it.
- **No ceiling.** A job whose runs take longer than its interval increments
`PendingRuns` on every missed occurrence forever. The job then runs
back-to-back indefinitely, and there is no bound, no warning, and nothing in
the UI that shows the queue depth.
Fix: clear `PendingRuns` in `SetGlobalPause(true)` and in `SetEnabled(false)`,
and cap it (a small constant, or the number of occurrences in one interval).
Document the cap in STANDARDS next to the existing overlap-policy entry, and
show the depth in the details pane if it is capped.
### 6.4 `normalizeJobs` never resolves duplicate IDs — low/medium
[store.go:180](../src/storage/store.go) assigns an ID only when one is absent
(`job.ID <= 0`). A hand-edited `jobs.json` — a workflow the project explicitly
supports and README documents — with two entries carrying `"id": 5` produces two
jobs that share one `JobRuntime` entry, one schedule-cache entry, and one
`SeedStats` bucket. `findByIDLocked` returns the first, so editing or deleting
one silently targets the other; both runs write their state onto the same
runtime.
Fix: track seen IDs during normalization and reassign the later duplicate, which
is exactly what the existing `next` counter already computes.
### 6.5 Log file names collide within the same second — low
[logfile.go:25](../src/runner/logfile.go) builds
`20060102-150405_<name>.log`. Two runs of the same job in the same second — a
fast job re-run manually, or a queue drain of a sub-second command — write the
same path and the second silently overwrites the first. `SeedStats` counts files,
so the run history also under-counts. Add a disambiguating suffix when the path
already exists.
### 6.6 Two different averages for the same history — low
[updateStats](../src/app/run.go) keeps a truncating incremental mean
(`(avg*(n-1) + d) / n` in integer arithmetic, so the truncation error
compounds), while [aggregateLogStats](../src/runner/seed.go) computes an exact
`sum / count`. The same run history therefore reports a different average
depending on whether it was seeded from logs at startup or accumulated live —
and the two are mixed, because seeds are the starting values that `updateStats`
then folds new runs into. Keep a running sum on `JobRuntime` and divide on
read.
### 6.7 Absolute paths are not cleaned; relative ones are — low
[ResolveConfiguredPath](../src/storage/store.go) returns an absolute path
verbatim and only `Clean`s the relative case. `UpdateSettings` decides whether
the user is switching jobs files by comparing the resolved path to
`Paths.JobsPath` as strings, so `C:/data/jobs.json` and `C:\data\jobs.json` read
as two different files and trigger the adoption branch against the file the app
is already using. `filepath.Clean` on both sides fixes it.
### 6.8 Missed occurrences during downtime are dropped — challenge, not a bug
`adoptJobsLocked` computes each job's first `NextDue` from `time.Now()`, so
occurrences that fell while the app was closed never run and never appear in
History. This is the right default for a desktop scheduler, but it is not
written down anywhere — a user coming from cron with `anacron` habits will
assume the opposite. Add it to STANDARDS §Intentional behavior.
---
## 7. Legacy code and migrations
The file-compatibility discipline STANDARDS describes is genuinely followed:
`Config.JobsDir``Config.JobsFile` and the retired `"default"` theme value are
both converted on load, cleared, and covered by a `storage` test. Nothing found
that reads a shape the app cannot write. The findings are smaller.
### 7.1 `domain.RunRecord` carries dead `yaml:` tags — low
[record.go](../src/domain/record.go) tags all nine fields `yaml:"…"`. Nothing
serialises the type — History is session-only and log files are written as
hand-rolled text — and there is no YAML dependency in `go.mod`. Leftover from an
earlier format. Delete them, or convert to `json:` if the type is ever meant to
be persisted.
### 7.2 Two compatibility shims with no retirement plan — low
`Config.JobsDir` (pre-0.15) and `Theme == "default"` (pre-1.0.1) are both
read-only shims that rewrite the file into the current shape on the next save,
so each one becomes dead the moment a user's config has been saved once by a
current build. Neither has a note saying when it can go. Add "remove after
<version>" to each, or a single ROADMAP entry that retires both.
### 7.3 `autostart` exposes two public surfaces for one job — low
Each of the three implementations exports both the `Manager` methods and the
bare `SetAutostart` / `AutostartStatus` functions the methods delegate to. Only
the interface is used outside the package (plus the tests). Unexport the
functions.
---
## 8. Stubs and claimed-but-unimplemented behavior
### 8.1 "Cleanup disabled" is documented and tested but unreachable — medium
`CleanupLogs` documents `maxFiles <= 0` and `maxAgeDays <= 0` as "policy
disabled", and `TestCleanupLogsZeroLimitsDisableBothPolicies` pins it. The app
can never produce that state: `validateConfig` rejects both as
"must be a positive number" ([operations.go:469](../src/app/operations.go)), and
`loadOrCreateConfig` backfills 0 to 100 / 30 on load
([store.go:119](../src/storage/store.go)). So a user cannot turn log cleanup off
at all, by GUI or by hand-editing.
This is also inconsistent with `DefaultTimeoutSeconds`, where the project went
to real trouble — a pointer type, a documented three-state table, a dedicated
test — precisely so that a meaningful zero would survive.
Decide one way: either accept 0 as "unlimited" in `validateConfig` and stop
backfilling it (documented in STANDARDS alongside the timeout rule), or delete
the unreachable branch in `CleanupLogs` and its test. The first is the better
outcome — "keep everything" is a real thing to want from a log retention
setting.
### 8.2 Window-size preferences are read but never written — low
`prefs.FloatWithFallback("window.width", …)` in
[run.go:64](../src/ui/run.go) always returns the fallback because no code path
writes those keys. Dead reads plus a comment that claims otherwise (4.3).
Replace with the constants and leave a one-line pointer to the frozen ROADMAP
item.
### 8.3 `notify-timing.log` shares the retention budget of run logs — low
[appendNotificationTimingLog](../src/ui/notify_timing.go) writes into
`logs_dir` with a `.log` extension, so `CleanupLogs` counts it against
`MaxLogFiles` and will delete it once it ages past `MaxLogAgeDays`. It is
diagnostic instrumentation shipped in 1.0.2 for the "Faster Windows failure
notifications" ROADMAP item, with no note on when it comes out. Give it a
different extension (or a `diagnostics/` subdirectory — `CleanupLogs` already
skips directories) and add its removal to that ROADMAP entry.
Items 1.2 (`SchedulerStateChanged`) and 2.3 (`collectActivity`) also belong to
this section; they are written up above.
---
## 9. GUI: crutches and layout
**This section is close to clean, and that is the headline.** The rule in
STANDARDS — "a size that must follow the theme is measured at build time, not
written as a pixel constant" — is actually observed: `rowOverlap`,
`captionColumnWidth`, `textColumnWidth`, `activityRowsHeight`, and
`initialSplitOffset` all derive from the theme or from measured text, and the
`ui` tests assert the resulting geometry under two themes. The two raw numbers
that remain (`commandOutputScroll`'s 460×70 minimum and the `+1` rounding
allowance in `activityRowsHeight`) both carry a comment explaining why nothing
about them tracks the theme. No layout crutches found.
The remaining items are small.
### 9.1 `AutostartStatus` runs PowerShell synchronously on the UI thread — low
`settingsView` is constructed eagerly during `newMainView`, and its constructor
calls `refreshAutostartStatus()``svc.AutostartStatus()`. On Windows with
autostart enabled that reaches `readShortcut`
([autostart_windows.go:124](../src/platform/autostart/autostart_windows.go)),
which spawns `powershell.exe` and blocks on `CombinedOutput()` — the same
PowerShell cold start ROADMAP measures at 700900 ms for notifications. It runs
before the window is shown, and again on every toggle of either checkbox.
Given the project already measures and cares about startup time
([PERFORMANCE.md](PERFORMANCE.md)), this is worth moving to a goroutine that
posts its result back through `fyne.Do`, with the label showing "Checking…"
meanwhile.
### 9.2 Two package-level mutable globals in `tray.go` — low
`mainWindowHidden` is justified and documented (Fyne exposes no
`Window.Visible`). `systemTrayRegistered` is not: it is process-global state
that no test can reset, and it exists only because `applyTrayBehavior` is called
from two places. Passing it, or hanging both flags off a small struct owned by
`Run`, removes the hidden coupling.
### 9.3 The activity list is refreshed twice per redraw — low
`refreshView` calls `dp.logs.Refresh()` immediately after `updateDetails`, which
already ends in `d.logs.Refresh()` ([jobs_view.go:91](../src/ui/jobs_view.go),
[jobs_view_details.go:103](../src/ui/jobs_view_details.go)). Harmless, but it is
the shape of duplicate-refresh bug that `TestToolbarButtonRedrawsRowAndDetails`
was written to prevent.
### 9.4 The folder-filter rebuild is repeated three times — low
`folderSelect.Options = folderOptions(jobs); folderSelect.Refresh()` appears
verbatim in the create, edit, and delete handlers. One `rebuildFolders()`
closure beside `refreshView`.
---
## 10. Under-documented contentious decisions
REVIEW §8 asks whether a decision a future reader would question has its
reasoning recorded. Most do — the platform layer, the timeout pointer, the
details-pane width coupling, and the frozen window-size work are all model
entries. These four are not.
- **Single-instance falls back to "start anyway."**
[singleinstance.go:34](../src/ui/singleinstance.go) documents *why* it does
not abort when port 37653 is held by something that is not GoSentry. It does
not document the consequence: two GoSentry processes then run two schedulers
against the same `jobs.json` and the same logs directory, each overwriting the
other's saves. Combined with 6.2 that is a plausible way to lose the file.
- **The instance channel is an unauthenticated localhost TCP port.** Any local
process, including one running as another user on a shared machine, can send
`show`. Low impact — the command only raises a window — but it is a design
choice, not an accident, and it should say so.
- **The nested `fyne.Do`** (5.4).
- **No catch-up after downtime** (6.8).
---
## 11. Other improvement proposals
- **Startup parses `gosentry.json` twice**, and `PeekKeepRunningInTray`
([store.go:22](../src/storage/store.go)) *creates* the file as a side effect
of a function named "Peek", before `OpenStore` runs. Harmless today; a
surprising name for a function with a write.
- **`-race` wall time is 4 minutes**, 229 s of it `src/ui`. That is the single
biggest tax on iteration in this repo and the reason the model
recommendations below lean toward first-pass correctness.
- **`scripts/test.bat` prints `✓` / `✗`** as UTF-8 in a file `cmd.exe` reads in
the OEM code page, so the summary lines render as mojibake on a default
Russian or US console. Use ASCII, or `chcp 65001`.
- **`dist/` in the working tree holds a 1.0.1 binary and 130 sample run logs.**
Correctly gitignored, so this is only a note: the stale binary next to a 1.0.2
source tree is an easy thing to hand someone by accident.
---
## Suggested order
Grouped so that each commit is independently reviewable and each medium finding
lands with its regression test.
1. **4.1 — restore `TestJobListViewIsCompact`, retire the other row.** Smallest,
and it restores an enforcement STANDARDS calls mandatory. Do it first so the
rest of the work runs against a suite that is honest about itself. TESTS.md
changes in the same commit: add the restored test back, drop the
`TestDefaultConfigUsesDetailedJobList` row, add the `notify_timing_test.go`
table (4.2), and add `PeekKeepRunningInTray` to the deliberate-0% list if
that is the intent.
2. **6.1 — the Windows quoting bug.** Self-contained, one function, CGO-free
package, four-row table test already written out above.
3. **6.2 — atomic writes**, plus a `Service.Stop()` on shutdown. Touches one
helper and one call site; protects everything else.
4. **2.1 — drop the three no-op `SaveJobs` calls**, and retire
`TestStartRunLockedRollbackOnSaveFailure` with the rollback it guards. Best
done after 6.2, so the durability question is already settled and this is
purely a removal.
5. **6.3 — `PendingRuns` lifecycle and cap**, with STANDARDS updated alongside.
6. **3.1 — cap History and stop rescanning column widths.** The biggest
behavioural win; needs a before/after measurement, and it is in `ui`, so it
is the item with the slowest feedback loop.
7. **3.2, 3.3 — I/O off `mu`, StartOnly context.** Related concurrency
cleanups; one commit each.
8. **8.1 — decide what a zero retention limit means**, and make the code, the
validator, and STANDARDS agree.
9. **1.1 — typed Service accessors, retire `Store()`.** Mechanical once decided,
but it touches eight UI sites and is best done when nothing else is in
flight. Rolls up 1.2, 1.3 and 7.3.
10. **5.1, 5.2 — the Jobs view state extraction**, folded into the ROADMAP
file-split item rather than done separately. 5.2 is a real defect, so if the
split slips, fix the selection-by-ID part on its own.
11. **The remaining low items** (2.2, 2.3, 3.43.6, 4.34.7, 6.46.7, 7.17.3,
8.2, 8.3, 9.19.4, §10, §11) as a small number of themed cleanup commits.
CHANGELOG entries are needed for 6.1, 6.2, 6.3, 3.1, 3.3, and 8.1 — those change
shipped behavior. The rest is internal.
---
## Which model to use
For running these items in Claude Code. As in the retired test-suite plan, the
deciding factor is **not** task size — it is that the feedback loop is slow: the
`ui` package needs the MSYS2 UCRT64 toolchain with CGO on, and `src/ui` alone
took **229 s** in this review's `go test -race ./...` run — every other package
in the tree finished in under 8 s. A model that gets an edit right on the first
pass is worth more than a faster one that needs a second four-minute build to
discover it was wrong.
| Item | Model | Why |
|---|---|---|
| 1 — restore one test, sync TESTS.md | **Haiku 4.5** (`claude-haiku-4-5`) | The test is recoverable verbatim from `git show 5b0e6fe^:src/domain/config_test.go`, and both judgment calls — that the removal was accidental, and that only one of the two is worth restoring — are already settled in §4.1. What is left is a paste plus four doc-table edits, in `domain`, which runs in ~2 s. Nothing to weigh. |
| 2 — Windows quoting | **Sonnet 5** (`claude-sonnet-5`) | The defect and the four expected outputs are already pinned in this document, so the judgment is made; writing the earliest-match-at-a-boundary scan and its table test is careful execution work. `runner` needs no CGO and its Windows-gated test file runs in seconds. |
| 3 — atomic writes + `Service.Stop()` | **Sonnet 5** | Temp-file-then-rename is a known pattern; the only real decisions (same directory, `Sync` before rename, what to do with a leftover `.tmp`) are stated. The `Stop()` wiring in `run.go` is two lines. |
| 4 — remove the no-op saves | **Opus 5** (`claude-opus-5`) | This one is a judgment call disguised as a deletion. It requires re-deriving, against the current `domain.Job`, that no durable field changes on those paths — and being willing to say "actually one does" instead of deleting the safety net. It also retires an existing regression test, which is the sort of change that should not be made by a model optimising for completing the task. |
| 5 — `PendingRuns` lifecycle and cap | **Opus 5** | Interacting state across pause, disable, drain, and the tick loop, with three existing queue tests that must keep passing and a cap whose value is a design decision, not a lookup. `app` is CGO-free, but the reasoning is the cost here, not the build. |
| 6 — History cap + incremental column widths | **Opus 5** | The item with the worst feedback loop (in `ui`, 229 s per attempt) and the one where a plausible-looking fix can be wrong: widths must never shrink below what is on screen, and the cap interacts with the sort toggle and the cached `rows` snapshot that `TestHistorySortToggleKeepsRowsInSync` exists to protect. Fast mode (`/fast`) is worth enabling here specifically, since the wait is real. |
| 7 — I/O off `mu`, StartOnly context | **Opus 5** | Lock-scope changes are exactly where a confident-but-wrong edit is expensive: moving `CleanupLogs` out from under `mu` must not move the snapshot reads with it. The StartOnly half requires knowing why `exec.CommandContext` keeps a goroutine alive without `Wait` — reasoning about the standard library's internals, not about this repo. |
| 8 — zero retention limits | **Sonnet 5** | Once the direction is chosen (accept 0 as unlimited, per §8.1), the change is a validator branch, a load branch, a STANDARDS entry, and two tests, all in CGO-free packages. If the decision goes the other way — deleting the branch and its test — it is smaller still. |
| 9 — typed Service accessors | **Sonnet 5** | Eight mechanical call-site conversions plus two new accessors. The design is settled in §1.1; the work is breadth, not depth. Half the sites are in `ui`, so budget one slow verification run rather than several. |
| 10 — Jobs view state extraction | **Opus 5** | The ROADMAP already says why: a split reads as pure movement while quietly dropping a function, and this one has to break up a constructor rather than move whole functions. The selection-by-ID defect has to survive the move as a fix, not be re-introduced by it. |
| 11 — the low-severity cleanups | **Sonnet 5**, or **Haiku 4.5** for the doc-only ones | Each is small and independently verifiable. Group the CGO-free ones (`domain`, `storage`, `runner`, `app`) into one pass and the `ui` ones into another, so the 229 s build is paid once rather than per item. |
Two notes on this table:
- **Sonnet 5 is the reasonable single choice** if you would rather not switch
models per item: items 47 and 10 are the only ones that really reward the
step up, and of those only 6 and 7 are likely to go wrong quietly. Sonnet 5's
introductory pricing runs through **2026-08-31** ($2/$10 per MTok vs $3/$15
after), against Opus 5's $5/$25.
- **Fast mode is available on Opus 5** (toggle with `/fast`). It is the same
model with higher output throughput, not a downgrade, but it bills at $10/$50,
so it only pays for itself when you are actually waiting on output. On this
plan that is item 6 — and, if you batch them, the `ui` half of item 11.
+46 -21
View File
@@ -18,15 +18,35 @@ machine):** average **773 ms** per toast (695874 ms), dominated by PowerShell
cold start. Re-run the script when comparing after a native toast implementation. cold start. Re-run the script when comparing after a native toast implementation.
**App-side timing:** each failure notification appends one line to **App-side timing:** each failure notification appends one line to
`logs/notify-timing.log` (`ms_after_run`, `ms_fyne_do`, `ms_send`, `logs/notify-timing.tsv` (`ms_after_run`, `ms_fyne_do`, `ms_send`,
`ms_app_total`). These columns end when Fyne returns from `SendNotification`; OS `ms_app_total`). These columns end when Fyne returns from `SendNotification`; OS
toast latency is not included. toast latency is not included. The `.tsv` extension keeps it out of
`runner.CleanupLogs`, which only manages `.log` files — this file is
diagnostic instrumentation for this item, not job output, and should be
removed (or unified with the run-log retention policy under its own knob) once
the native-toast direction below lands and the timing data is no longer
needed.
**Direction:** add `src/platform/notify/` with a native Windows toast (WinRT or **Direction:** add `src/platform/notify/` with a native Windows toast (WinRT or
a maintained Go wrapper), used for failure notifications on Windows. Keep Fyne a maintained Go wrapper), used for failure notifications on Windows. Keep Fyne
`SendNotification` on Linux (DBus / xdg-desktop-portal) unless profiling shows it `SendNotification` on Linux (DBus / xdg-desktop-portal) unless profiling shows it
needs the same treatment. needs the same treatment.
### Retire the config compatibility shims
Two read-only shims in `storage.loadOrCreateConfig` rewrite an old file into
the current shape on the next save, so each becomes dead the moment a user's
config has been saved once by a build that has it:
- `Config.JobsDir` (pre-0.15, superseded by `Config.JobsFile`).
- `Theme == "default"` (pre-1.0.1, superseded by `ThemeSystem`).
Neither has an expiry. Remove both — the field, the migration branch, and
`TestLoadOrCreateConfigMigratesJobsDir` /
`TestLoadOrCreateConfigMigratesLegacyThemeDefault` — once a release has shipped
long enough that a config file still carrying either old shape is not a
realistic upgrade path GoSentry needs to support.
### Dynamic tray icon toggle ### Dynamic tray icon toggle
Fyne exposes `SetSystemTrayIcon` and related APIs only at application startup. Fyne exposes `SetSystemTrayIcon` and related APIs only at application startup.
@@ -123,24 +143,25 @@ Design notes / open questions:
[ARCHITECTURE.md](ARCHITECTURE.md) sets a ~250-line guideline per source file [ARCHITECTURE.md](ARCHITECTURE.md) sets a ~250-line guideline per source file
and records the `jobs_view.go` and `settings_view.go` splits as the worked and records the `jobs_view.go` and `settings_view.go` splits as the worked
examples. Six non-test files are over it at 1.0.0, including both files that examples. `jobs_view.go` was split again in 1.0.3 — into view, state, list, and
were already split once: toolbar — because the selection defect it carried was a symptom of the size
(one 330-line constructor over seven shared locals). Six non-test files are
over the guideline:
| File | Lines | | File | Lines |
|------|-------| |------|-------|
| `src/app/operations.go` | 490 | | `src/app/operations.go` | 529 |
| `src/ui/jobs_view.go` | 355 | | `src/storage/store.go` | 382 |
| `src/app/run.go` | 287 | | `src/ui/history_view.go` | 355 |
| `src/ui/history_view.go` | 282 | | `src/ui/settings_view.go` | 326 |
| `src/ui/settings_view.go` | 277 | | `src/app/run.go` | 275 |
| `src/storage/store.go` | 265 | | `src/app/service.go` | 252 |
This is deliberately deferred to the next whole-project review rather than done The remaining six are deliberately deferred rather than done piecemeal: a
piecemeal: a future review already asks item 2 to look for exactly this, split touches every reader of the file, and doing them in one pass keeps the
a split touches every reader of the file, and doing all six in one pass keeps seams consistent instead of settling each one its own way. Splitting is
the seams consistent instead of settling them six different ways. Splitting is
also the kind of change that reads as pure movement while quietly dropping a also the kind of change that reads as pure movement while quietly dropping a
function, so it wants one careful pass, not six hurried ones. function, so it wants one careful pass, not a hurried one per file.
Seams visible today, as a starting point rather than a decision: Seams visible today, as a starting point rather than a decision:
@@ -152,13 +173,17 @@ Seams visible today, as a starting point rather than a decision:
- **`history_view.go`** — the column-measuring helpers (`textWidth` through - **`history_view.go`** — the column-measuring helpers (`textWidth` through
`historyColumnWidths`) are pure, already unit-tested, and independent of the `historyColumnWidths`) are pure, already unit-tested, and independent of the
table they size. table they size.
- **`jobs_view.go`** — nearly all of it is one `newJobsView` constructor, so the - **`store.go`** — path resolution, the config load/normalize path, and the jobs
split has to break that function up (list template, toolbar handlers, load/normalize path are three separate concerns in one file.
assembly) rather than move whole functions. Larger judgement call than the - **`run.go`**, **`settings_view.go`**, **`service.go`** — barely over. Worth
others.
- **`run.go`**, **`settings_view.go`**, **`store.go`** — barely over. Worth
re-measuring at the time; if a pass elsewhere has shrunk them, leave them re-measuring at the time; if a pass elsewhere has shrunk them, leave them
alone rather than splitting for the sake of the number. alone rather than splitting for the sake of the number. The counts above move
a few lines either way with any edit, so re-measure before acting on them
rather than treating the table as current.
The `jobs_view.go` pass is the worked example for the rest: the constructor was
broken up along the state it shared, not along line count, and the split landed
with the selection fix rather than promising it separately.
Scope note: the guideline is about source files. Test files are much larger and Scope note: the guideline is about source files. Test files are much larger and
that is fine — a table-driven test file grows with the cases it covers. that is fine — a table-driven test file grows with the cases it covers.
+41
View File
@@ -11,6 +11,15 @@ in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
- Fixes with severity ≥ medium → regression test. - Fixes with severity ≥ medium → regression test.
- Documented intentional behavior → section below, not a backlog bug. - Documented intentional behavior → section below, not a backlog bug.
- UI view constructors accept `*app.Service`; call `app.Open()` only from `run.go`. - UI view constructors accept `*app.Service`; call `app.Open()` only from `run.go`.
- **No blocking file I/O under `Service.mu`.** It is the lock the Fyne main
thread takes on every `Jobs()` and `Runtime()` call, so a JSON write, a
log-directory scan, or a pass over every log header inside it makes a UI
refresh wait on the disk. Mutate state under the lock, snapshot what the I/O
needs, and run the I/O after `mu.Unlock()` — the way `emit()` already is.
Store writes go through `Service.deferSaveLocked` and `Store.PrepareSaveJobs` /
`Store.PrepareSaveConfig`, which take `saveMu` while `mu` is still held so
writes still reach the file in the order their snapshots were taken; log
cleanup and `runner.SeedStats` run from plain snapshots.
- A size that must follow the theme is **measured at build time, not written as - A size that must follow the theme is **measured at build time, not written as
a pixel constant.** `theme.Padding()` and text metrics depend on the running a pixel constant.** `theme.Padding()` and text metrics depend on the running
app's theme, text size, and DPI, so a hand-tuned number is only correct for app's theme, text size, and DPI, so a hand-tuned number is only correct for
@@ -62,6 +71,22 @@ change to their shape has to stay compatible on its own.
= 0) and is overridable per job (`Job.TimeoutSeconds *int`: unset = inherit the = 0) and is overridable per job (`Job.TimeoutSeconds *int`: unset = inherit the
global default, 0 = no timeout, positive = seconds). Neither zero may be global default, 0 = no timeout, positive = seconds). Neither zero may be
normalized away on load — 0 is a value, not a missing field. normalized away on load — 0 is a value, not a missing field.
- **`Config.MaxLogFiles` and `Config.MaxLogAgeDays` of 0 mean "keep everything",
not "unset".** `runner.CleanupLogs` already treated `<= 0` as "policy
disabled"; `app.validateConfig` and the Settings form now accept 0 (only a
negative count is rejected), and `storage.loadOrCreateConfig` no longer
backfills 0 to 100 / 30 — a config written before either field existed still
picks up the default because `json.Unmarshal` leaves an absent key holding
whatever `DefaultConfig()` set, the same mechanism `DefaultTimeoutSeconds`
relies on.
- **A `StartOnly` process is expected to outlive GoSentry.** The option exists to
launch something and let go of it, so the runner builds that invocation on
`context.Background()`, not on the application's lifecycle context: quitting
GoSentry (or cancelling a run) does not stop a process it started this way, and
`Service.Stop()` reaches only jobs the runner is still waiting on. The
uncancelable context is also what keeps `os/exec` from leaving a watcher
goroutine per run — it only starts one when the context can be done, and
`StartOnly` never calls `Wait` to end it.
- **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the - **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the
current process. Log files on disk feed aggregate statistics via `SeedStats` current process. Log files on disk feed aggregate statistics via `SeedStats`
only. See [ARCHITECTURE.md](ARCHITECTURE.md). only. See [ARCHITECTURE.md](ARCHITECTURE.md).
@@ -96,6 +121,22 @@ change to their shape has to stay compatible on its own.
an occurrence that fired before the pause/disable. The details pane appends an occurrence that fired before the pause/disable. The details pane appends
", N queued" to the statistics line via `DisplayStats` whenever the count is ", N queued" to the statistics line via `DisplayStats` whenever the count is
non-zero. non-zero.
- **Single-instance arbitration falls back to "start anyway" when the port is
held by something else.** `acquireSingleInstance` (`singleinstance.go`)
binds `127.0.0.1:37653`; if that fails and a dial to the same address does
not answer as GoSentry either, startup continues rather than refusing to
open because of an unrelated local listener. The consequence is deliberate
but worth spelling out: two GoSentry processes can then run two schedulers
against the same `jobs.json` and the same logs directory, each overwriting
the other's saves. Atomic writes (`writeFileAtomic`) prevent a *torn* file
from a concurrent write, but not one process's save clobbering the other's.
- **The single-instance channel is an unauthenticated localhost TCP port.**
Port 37653 accepts one command, `"show"`, from any local process — including
one running as a different user on a shared machine. This is a deliberate
scope choice, not an oversight: the command only raises the existing window,
so the impact of an unwelcome sender is a window popping up, not data
exposure or control. Anything with a larger blast radius on that channel
would need real authentication.
## Out of scope ## Out of scope
+85 -11
View File
@@ -26,6 +26,12 @@ The GUI tests build the Fyne desktop backend, so CGO must be enabled; on Windows
that means the MSYS2 UCRT64 toolchain described in that means the MSYS2 UCRT64 toolchain described in
[DEVELOPMENT.md](DEVELOPMENT.md). [DEVELOPMENT.md](DEVELOPMENT.md).
`src/ui` dominates `go test -race ./...`'s wall time — around 229s in the
2026-08-05 whole-project review, against under 8s for every other package
combined. Budget iteration accordingly: a change confined to `domain`,
`storage`, `runner`, `scheduler`, or `app` gets a fast feedback loop; a `ui`
change does not.
### Manual test commands ### Manual test commands
Run all tests: Run all tests:
@@ -60,11 +66,31 @@ exercised from another one's tests — `domain.NewRuntime`, for instance, is
covered by the `app` tests. Measure the engine packages together instead: covered by the `app` tests. Measure the engine packages together instead:
```bash ```bash
go test -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app go test -coverprofile=cover.out -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app
``` ```
That figure was 84.4% at the 2026-08-04 review, which is the number to compare In the PowerShell environment DEVELOPMENT.md prescribes on Windows, PowerShell
against before concluding that coverage has slipped. splits the comma-separated `-coverpkg` list on its own and the command fails
with `directory not found`. Use the stop-parsing token, or quote the whole
flag — and note that `--%` swallows the rest of the line, so the profile has to
be read by a second command:
```powershell
go test --% -coverprofile=cover.out -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app
```
The total is the last line of the profile summary. It is **not** any of the
per-package lines `go test` prints: with `-coverpkg` spanning five packages,
each of those reports only what that one package's tests reached across the
whole set, so all five are far below the real figure.
```powershell
go tool cover -func=cover.out | Select-Object -Last 1
```
That total was 84.4% at the 2026-08-04 review and 84.1% at the 2026-08-07
documentation audit — the number to compare against before concluding that
coverage has slipped.
--- ---
@@ -166,6 +192,8 @@ Tests all mutating operations on the Service, scheduler integration, and setting
| `TestUpdateSettingsAdoptsExistingJobsFile` | Verifies that selecting a jobs file that already exists replaces the job list with its contents, rebuilds runtimes, and emits `JobsLoaded`. | | `TestUpdateSettingsAdoptsExistingJobsFile` | Verifies that selecting a jobs file that already exists replaces the job list with its contents, rebuilds runtimes, and emits `JobsLoaded`. |
| `TestUpdateSettingsKeepsJobsWhenTheNewFileIsMissing` | Verifies that a path with no file behind it receives the current jobs instead (the rename/relocate case). | | `TestUpdateSettingsKeepsJobsWhenTheNewFileIsMissing` | Verifies that a path with no file behind it receives the current jobs instead (the rename/relocate case). |
| `TestUpdateSettingsRefusesJobsFileSwitchWhileRunning` | Verifies that switching the jobs file is refused (and not persisted) while a job runs, while unrelated settings still save. | | `TestUpdateSettingsRefusesJobsFileSwitchWhileRunning` | Verifies that switching the jobs file is refused (and not persisted) while a job runs, while unrelated settings still save. |
| `TestUpdateSettingsSeedsAdoptedJobsFromLogs` | Verifies that statistics reconstructed from the new logs directory still reach the runtime map, now that the log scan happens before `UpdateSettings` takes `mu`. |
| `TestConcurrentJobOperationsLeaveTheFileMatchingMemory` | Verifies that saves prepared under `mu` and run after it is released still land in mutation order, so `jobs.json` matches the in-memory list after concurrent create/disable operations. |
| `TestSetJobListViewPersistsToConfigFile` | Verifies the Jobs-list density preference reaches `gosentry.json`, so the chosen view reopens after a restart. | | `TestSetJobListViewPersistsToConfigFile` | Verifies the Jobs-list density preference reaches `gosentry.json`, so the chosen view reopens after a restart. |
| `TestSetJobListViewNormalizesUnknownValue` | Verifies anything but `"compact"` is stored as `"detailed"`, so the config never gains a value no reader understands. | | `TestSetJobListViewNormalizesUnknownValue` | Verifies anything but `"compact"` is stored as `"detailed"`, so the config never gains a value no reader understands. |
| `TestPrependLogCapsActivityList` | Verifies that the activity log never grows beyond its maximum cap. | | `TestPrependLogCapsActivityList` | Verifies that the activity log never grows beyond its maximum cap. |
@@ -221,8 +249,8 @@ Tests display-formatting helpers used by the UI.
| `TestStatusText` | Verifies that job status codes map to the correct display strings. | | `TestStatusText` | Verifies that job status codes map to the correct display strings. |
| `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. | | `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. |
| `TestEventLine` | Verifies the one-line activity rendering of a `RunRecord`, including the log basename and the `Unknown` fallback for a blank trigger. | | `TestEventLine` | Verifies the one-line activity rendering of a `RunRecord`, including the log basename and the `Unknown` fallback for a blank trigger. |
| `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". | | `TestDisplayFolder` | Verifies that an empty folder string shows "(No folder)". |
| `TestDisplayArguments` | Verifies that an empty arguments string shows "None". | | `TestDisplayArguments` | Verifies that an empty arguments string shows "(none)". |
| `TestDisplayRunMode` | Verifies run-mode labels for normal and start-only modes. | | `TestDisplayRunMode` | Verifies run-mode labels for normal and start-only modes. |
| `TestDisplayInvocation` | Verifies that the full invocation display string combines command and arguments with spacing. | | `TestDisplayInvocation` | Verifies that the full invocation display string combines command and arguments with spacing. |
| `TestDisplayIndex` | Verifies the list position of a job index in a filtered index slice. | | `TestDisplayIndex` | Verifies the list position of a job index in a filtered index slice. |
@@ -243,15 +271,19 @@ Tests JSON round-tripping, default generation, and backward compatibility.
| `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. | | `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. |
| `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. | | `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. |
| `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. | | `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. |
| `TestNormalizeJobsReassignsDuplicateIDs` | Verifies that a hand-edited `jobs.json` with two entries sharing one ID gets the later duplicates reassigned instead of colliding on one runtime. |
| `TestResolveConfiguredPathCleansAbsolutePaths` | Verifies (Windows only) that forward-slash and backslash spellings of the same absolute path resolve to the same string. |
| `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults. | | `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults. |
| `TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun` | Verifies that a missing jobs file is created with the sample jobs from `defaultJobs`. | | `TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun` | Verifies that a missing jobs file is created with the sample jobs from `defaultJobs`. |
| `TestLoadOrCreateConfigKeepsZeroTimeoutOnReload` | Verifies that `default_timeout_seconds: 0` survives a reload rather than being normalized away — 0 is a value, not a missing field. | | `TestLoadOrCreateConfigKeepsZeroTimeoutOnReload` | Verifies that `default_timeout_seconds: 0` survives a reload rather than being normalized away — 0 is a value, not a missing field. |
| `TestLoadOrCreateConfigPreservesZeroRetentionLimits` | Verifies that `max_log_files` / `max_log_age_days` of 0 read back as 0 ("keep everything") instead of being backfilled to the 100 / 30 defaults — a field the file sets is not the missing-field case. |
| `TestLoadOrCreateConfigMigratesJobsDir` | Verifies that a pre-0.15 `jobs_dir` becomes `jobs_file` pointing at the same `jobs.json`, and that the retired key is not written back. | | `TestLoadOrCreateConfigMigratesJobsDir` | Verifies that a pre-0.15 `jobs_dir` becomes `jobs_file` pointing at the same `jobs.json`, and that the retired key is not written back. |
| `TestLoadOrCreateConfigMigratesLegacyThemeDefault` | Verifies that a config storing the retired `"default"` theme value is normalized to `system` on load. | | `TestLoadOrCreateConfigMigratesLegacyThemeDefault` | Verifies that a config storing the retired `"default"` theme value is normalized to `system` on load. |
| `TestLoadJobsFileReportsMissingWithoutCreating` | Verifies that `LoadJobsFile` reports a missing file as not-found without creating or seeding it, and normalizes the jobs it does load. | | `TestLoadJobsFileReportsMissingWithoutCreating` | Verifies that `LoadJobsFile` reports a missing file as not-found without creating or seeding it, and normalizes the jobs it does load. |
| `TestApplyConfigPathsDerivesJobsDir` | Verifies that the configured jobs file resolves against the program folder and that `Paths.JobsDir` is derived from it. | | `TestApplyConfigPathsDerivesJobsDir` | Verifies that the configured jobs file resolves against the program folder and that `Paths.JobsDir` is derived from it. |
| `TestJobTimeoutRoundTripsThreeStates` | Verifies the on-disk encoding that keeps "inherit" and "no timeout" distinguishable: `nil` is omitted entirely, an explicit `0` is written and read back as set. | | `TestJobTimeoutRoundTripsThreeStates` | Verifies the on-disk encoding that keeps "inherit" and "no timeout" distinguishable: `nil` is omitted entirely, an explicit `0` is written and read back as set. |
| `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. | | `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. |
| `TestWriteJSONReplacesFileAtomically` | Pins the durability fix: `writeJSON` replaces the destination through a temp file and a rename rather than truncating it in place, and leaves no temp file behind. |
--- ---
@@ -319,6 +351,7 @@ Tests command execution, exit code handling, output capture, and the run timeout
|------|---------| |------|---------|
| `TestRunJobStartOnlyDoesNotWaitForExitCode` | Verifies that `StartOnly: true` jobs launch and return "OK" immediately without waiting for the process to exit. | | `TestRunJobStartOnlyDoesNotWaitForExitCode` | Verifies that `StartOnly: true` jobs launch and return "OK" immediately without waiting for the process to exit. |
| `TestRunJobStartOnlyReportsStartFailure` | Verifies that `StartOnly: true` jobs still report "Failed" if the process cannot be started. | | `TestRunJobStartOnlyReportsStartFailure` | Verifies that `StartOnly: true` jobs still report "Failed" if the process cannot be started. |
| `TestRunJobStartOnlyLeavesNoContextWatcher` | Verifies that a start-only run leaves no `os/exec` context-watcher goroutine behind, since it never calls `Wait` and the started process is meant to outlive the app. |
--- ---
@@ -335,6 +368,7 @@ Tests the Windows shell invocation and hidden-window flags.
| `TestShellCommandHidesWindow` | Verifies that shell commands request hidden-window startup to prevent console flash. | | `TestShellCommandHidesWindow` | Verifies that shell commands request hidden-window startup to prevent console flash. |
| `TestShellCommandUsesWindowsSafeQuoting` | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. | | `TestShellCommandUsesWindowsSafeQuoting` | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. |
| `TestWindowsShellCommandLineQuotesUnquotedProgramPath` | Verifies that unquoted program paths in shell commands are quoted while preserving already-quoted arguments. | | `TestWindowsShellCommandLineQuotesUnquotedProgramPath` | Verifies that unquoted program paths in shell commands are quoted while preserving already-quoted arguments. |
| `TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension` | Regression: the program path ends at the *earliest* extension match sitting at a token boundary — not the first extension in `.exe`/`.cmd`/`.bat`/`.com` list order, and not a substring inside another word — so a `.bat` wrapper followed by an `.exe` argument still quotes only the wrapper. |
--- ---
@@ -373,6 +407,19 @@ Tests log-file cleanup by age and by count.
--- ---
### src/runner/logfile_test.go
**Package:** `runner`
Tests the disambiguating suffix `writeRunLog` applies when two runs land on
the same second.
| Test | Purpose |
|------|---------|
| `TestUniqueLogPathAvoidsCollision` | Verifies repeated calls for the same file name return distinct paths instead of silently overwriting an existing log. |
---
### src/platform/autostart/autostart_windows_test.go ### src/platform/autostart/autostart_windows_test.go
**Location:** `src/platform/autostart/autostart_windows_test.go` **Location:** `src/platform/autostart/autostart_windows_test.go`
@@ -468,11 +515,34 @@ widgets are assembled.
| `TestJobListViewCompactConfigOpensCompact` | Verifies the persisted density is honoured at build time, not only after a tap. | | `TestJobListViewCompactConfigOpensCompact` | Verifies the persisted density is honoured at build time, not only after a tap. |
| `TestJobsSidebarWidthIsItsContent` | Regression guard: nothing but the sidebar's own toolbar row imposes a width floor on it. | | `TestJobsSidebarWidthIsItsContent` | Regression guard: nothing but the sidebar's own toolbar row imposes a width floor on it. |
| `TestJobsSplitOpensAtTheSidebarWidth` | Verifies the derived split offset opens the divider at the sidebar's own width at the default window size — enough that the toolbar is never born clipped, and no more. | | `TestJobsSplitOpensAtTheSidebarWidth` | Verifies the derived split offset opens the divider at the sidebar's own width at the default window size — enough that the toolbar is never born clipped, and no more. |
| `TestToolbarButtonRedrawsRowAndDetails` | Regression guard: with the duplicate refreshes removed from the handlers, `refreshView` alone must re-snapshot the jobs and repopulate the details pane. | | `TestToolbarButtonRedrawsRowAndDetails` | Regression guard: with the duplicate refreshes removed from the handlers, `jobsView.refresh` alone must re-snapshot the jobs and repopulate the details pane. |
| `TestJobsViewSelectionSurvivesAJobsFileSwitch` | Regression guard: adopting a different jobs file replaces the whole list from the Service, and the refresh that follows must leave the details pane and the list highlight describing the same job — not redraw the pane from a row index that belonged to the previous list. |
| `TestDetailCaptionWidthCoversEveryCaption` | Verifies every caption `metadataRows` returns fits the measured caption column, which is what makes the single row list self-enforcing. | | `TestDetailCaptionWidthCoversEveryCaption` | Verifies every caption `metadataRows` returns fits the measured caption column, which is what makes the single row list self-enforcing. |
--- ---
### src/ui/jobs_view_state_test.go
**Package:** `ui`
Tests `jobsViewState`, the Jobs tab's model: the job/runtime snapshot, the
folder filter, and the ID-based selection. No Fyne app is built — the state
touches no widgets, so these run in milliseconds.
| Test | Purpose |
|------|---------|
| `TestJobsViewStateSelectsTheFirstJob` | Verifies the opening state selects the first row, so the details pane is never blank when there is something to show. |
| `TestJobsViewStateEmptyListSelectsNothing` | Verifies an empty job list leaves nothing selected and no row to highlight (`displayRow` = -1). |
| `TestJobsViewStateSelectionFollowsTheJobNotTheRow` | Regression guard: a job removed above the selected one (through the Service, the way an external change reaches the view) must not slide the selection onto its neighbour — the selection is a job ID, and only its row moves. |
| `TestJobsViewStateDropsSelectionWhenItsJobIsGone` | Verifies a selection whose job no longer exists falls back to the first visible row instead of describing whichever job inherited its position. |
| `TestJobsViewStateApplyFilter` | Verifies the folder filter keeps a selection it still shows, moves it to the folder's first row when it does not, and that "No folder" matches the job without one. |
| `TestJobsViewStateEmptyFilterSelectsNothing` | Verifies a filter matching no job is a filter choice, not an error state: nothing selected, nothing highlighted, and the selection returns when the filter is cleared. |
| `TestJobsViewStateHiddenSelectionIsNotHighlighted` | Verifies a selected job the filter hides reports no display row rather than falling back to row 0, which would highlight an unrelated job. |
| `TestJobsViewStateRuntimeIsNeverNil` | Verifies `runtime` returns an empty `JobRuntime` for a job the Service has none for, so callers need no nil check. |
| `TestJobsViewStateJobAtRejectsRowsOutsideTheFilter` | Verifies row lookups are bounded by the filtered rows, which is what the list widget draws from. |
---
### src/ui/history_view_test.go ### src/ui/history_view_test.go
**Package:** `ui` **Package:** `ui`
@@ -482,8 +552,6 @@ column-width behaviour of the assembled table.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestCollectActivityMergesAndSorts` | Verifies per-job logs are merged and sorted by time. |
| `TestCollectActivitySkipsMissingRuntimes` | Verifies missing runtime entries are skipped safely. |
| `TestHistoryCellText` | Verifies table cell text for all columns; empty trigger → `Unknown`. | | `TestHistoryCellText` | Verifies table cell text for all columns; empty trigger → `Unknown`. |
| `TestLogFileName` | Verifies log path basename extraction on Windows and Unix paths. | | `TestLogFileName` | Verifies log path basename extraction on Windows and Unix paths. |
| `TestNewEventUsesConsistentTimestampShape` | Verifies UI events use the same timestamp layout as run records. | | `TestNewEventUsesConsistentTimestampShape` | Verifies UI events use the same timestamp layout as run records. |
@@ -567,7 +635,7 @@ Tests the failure-notification timing diagnostics added in 1.0.2.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestNotificationTimingFormatLine` | Verifies `notificationTiming.formatLine` renders the job name and the three millisecond deltas (`ms_after_run`, `ms_fyne_do`, `ms_send`) plus their sum (`ms_app_total`). | | `TestNotificationTimingFormatLine` | Verifies `notificationTiming.formatLine` renders the job name and the three millisecond deltas (`ms_after_run`, `ms_fyne_do`, `ms_send`) plus their sum (`ms_app_total`). |
| `TestAppendNotificationTimingLogWritesHeaderAndRow` | Verifies `appendNotificationTimingLog` creates `notify-timing.log` with its header on first write and appends a row containing the job name. | | `TestAppendNotificationTimingLogWritesHeaderAndRow` | Verifies `appendNotificationTimingLog` creates `notify-timing.tsv` with its header on first write and appends a row containing the job name. |
--- ---
@@ -620,6 +688,12 @@ A coverage run over the non-UI packages reports these as uncovered. All are
intentional; none is an oversight to be "fixed" with a test. intentional; none is an oversight to be "fixed" with a test.
- The real `Clock` — a fake is injected everywhere it is used. - The real `Clock` — a fake is injected everywhere it is used.
- `storage.OpenStore`, `storage.ResolvePaths`, `storage.PeekKeepRunningInTray`, `app.Service.Start`, `app.Service.Open` — process entry points, exercised by running the app. - `storage.OpenStore`, `storage.ResolvePaths`, `storage.PeekKeepRunningInTray`, `app.Service.Start`, `app.Open` — process entry points, exercised by running the app.
- The autostart and desktop-icon wrappers — OS integration, driven only on a real desktop. - The autostart and desktop-icon wrappers (`app.Service.InstallDesktopIcon`, `AutostartStatus`, `ApplyAutostart`) — OS integration, driven only on a real desktop.
- `app.Service.ShouldNotifyOnFailure` — a getter under the mutex. - `app.Service.ShouldNotifyOnFailure` — a getter under the mutex.
- `app.Service.Config` and `app.Service.Paths` — read only from `src/ui`, which
this run excludes, so they are covered by the suite but not by this profile.
The same applies to `storage.Store.SaveJobs`: the engine writes through
`PrepareSaveJobs`, and the one-shot wrapper is what `OpenStore` uses.
- The five `isEvent` marker methods in `app/events.go` — empty bodies that exist
only to close the `Event` interface.
Binary file not shown.

After

Width:  |  Height:  |  Size: 37 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 99 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 100 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 65 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 57 KiB

+7
View File
@@ -6,6 +6,13 @@ REM Runs go vet and go test with race detection
REM Move to repository root REM Move to repository root
cd /d "%~dp0\.." cd /d "%~dp0\.."
REM This file is UTF-8 (the ✓/✗ below). cmd.exe reads batch files in the
REM console's active code page, which defaults to the system locale (e.g.
REM CP866 on Russian Windows) rather than UTF-8, so without this the two
REM symbols render as mojibake. Switching the console to UTF-8 first fixes
REM that; >nul silences chcp's own "Active code page" confirmation line.
chcp 65001 >nul
REM Fyne uses native libraries through CGO. MSYS2 UCRT64 provides the GCC toolchain REM Fyne uses native libraries through CGO. MSYS2 UCRT64 provides the GCC toolchain
REM expected by the Windows build; prepending it keeps the script self-contained REM expected by the Windows build; prepending it keeps the script self-contained
REM without permanently changing the user's system PATH. REM without permanently changing the user's system PATH.
+6 -3
View File
@@ -4,9 +4,12 @@ import "gitea.mixdep.ru/mix/gosentry/src/domain"
// Event is something the Service did to its state that observers may want to // Event is something the Service did to its state that observers may want to
// react to. It is a sealed interface: the concrete types in this file are the // react to. It is a sealed interface: the concrete types in this file are the
// only implementations (enforced by the unexported isEvent marker), so a UI // only implementations (enforced by the unexported isEvent marker), so an
// listener can exhaustively type-switch over them and the compiler will flag a // Event handed to an Observer is always one of the types declared here — a
// new event type that a switch forgot to handle. // caller outside this package cannot manufacture a new one. Go's type switch
// has no exhaustiveness check, so sealing buys that guarantee, not a
// compile-time warning when a new event type is added and a listener forgets
// to handle it; the listener still has to be updated by hand.
// //
// Events replace the old single onChange callback. Instead of the scheduler // Events replace the old single onChange callback. Instead of the scheduler
// reaching into the GUI, the Service emits typed events and the UI subscribes — // reaching into the GUI, the Service emits typed events and the UI subscribes —
+66 -35
View File
@@ -43,15 +43,23 @@ func (s *Service) CreateJob(job domain.Job) (domain.Job, error) {
s.parseScheduleLocked(&job) s.parseScheduleLocked(&job)
record := uiRecord(job.ID, job.Name, "Created", "Job was added") record := uiRecord(job.ID, job.Name, "Created", "Job was added")
prependLog(runtime, record) prependLog(runtime, record)
err := s.store.SaveJobs(s.jobs) save := s.deferSaveLocked(s.store.PrepareSaveJobs(s.jobs))
if err != nil { s.mu.Unlock()
s.jobs = s.jobs[:len(s.jobs)-1]
if err := save(); err != nil {
// The write is atomic, so a failure left the file holding the previous
// list: take the job back out so memory matches what is on disk. Another
// operation may have run in between, so it is removed by ID rather than by
// truncating the slice.
s.mu.Lock()
if index := s.indexByIDLocked(job.ID); index >= 0 {
s.jobs = append(s.jobs[:index], s.jobs[index+1:]...)
}
delete(s.runtimes, job.ID) delete(s.runtimes, job.ID)
delete(s.schedules, job.ID) delete(s.schedules, job.ID)
s.mu.Unlock() s.mu.Unlock()
return domain.Job{}, err return domain.Job{}, err
} }
s.mu.Unlock()
s.emit(RunRecorded{Record: record}) s.emit(RunRecorded{Record: record})
s.emit(JobChanged{JobID: job.ID}) s.emit(JobChanged{JobID: job.ID})
return job, nil return job, nil
@@ -87,10 +95,10 @@ func (s *Service) UpdateJob(job domain.Job) error {
s.refreshNextRunLocked(existing, runtime) s.refreshNextRunLocked(existing, runtime)
record := uiRecord(job.ID, job.Name, "Updated", "Job settings changed") record := uiRecord(job.ID, job.Name, "Updated", "Job settings changed")
prependLog(runtime, record) prependLog(runtime, record)
err := s.store.SaveJobs(s.jobs) save := s.deferSaveLocked(s.store.PrepareSaveJobs(s.jobs))
s.mu.Unlock() s.mu.Unlock()
if err != nil { if err := save(); err != nil {
return err return err
} }
s.emit(RunRecorded{Record: record}) s.emit(RunRecorded{Record: record})
@@ -113,10 +121,10 @@ func (s *Service) DeleteJob(id int) error {
delete(s.runtimes, id) delete(s.runtimes, id)
delete(s.schedules, id) delete(s.schedules, id)
record := uiRecord(id, deleted.Name, "Deleted", "Job was removed") record := uiRecord(id, deleted.Name, "Deleted", "Job was removed")
err := s.store.SaveJobs(s.jobs) save := s.deferSaveLocked(s.store.PrepareSaveJobs(s.jobs))
s.mu.Unlock() s.mu.Unlock()
if err != nil { if err := save(); err != nil {
return err return err
} }
s.emit(RunRecorded{Record: record}) s.emit(RunRecorded{Record: record})
@@ -155,10 +163,10 @@ func (s *Service) SetEnabled(id int, enabled bool) error {
record = uiRecord(id, job.Name, "Paused", "Job was disabled") record = uiRecord(id, job.Name, "Paused", "Job was disabled")
} }
prependLog(runtime, record) prependLog(runtime, record)
err := s.store.SaveJobs(s.jobs) save := s.deferSaveLocked(s.store.PrepareSaveJobs(s.jobs))
s.mu.Unlock() s.mu.Unlock()
if err != nil { if err := save(); err != nil {
return err return err
} }
s.emit(RunRecorded{Record: record}) s.emit(RunRecorded{Record: record})
@@ -188,10 +196,10 @@ func (s *Service) SetGlobalPause(paused bool) error {
} }
s.refreshNextRunFromLocked(job, runtime, now) s.refreshNextRunFromLocked(job, runtime, now)
} }
err := s.store.SaveConfig() save := s.deferSaveLocked(s.store.PrepareSaveConfig())
s.mu.Unlock() s.mu.Unlock()
if err != nil { if err := save(); err != nil {
return err return err
} }
state, detail := "Resumed", "All job execution resumed" state, detail := "Resumed", "All job execution resumed"
@@ -219,9 +227,9 @@ func (s *Service) SetJobListView(view domain.JobListView) error {
return nil return nil
} }
s.store.Config.JobListView = view s.store.Config.JobListView = view
err := s.store.SaveConfig() save := s.deferSaveLocked(s.store.PrepareSaveConfig())
s.mu.Unlock() s.mu.Unlock()
return err return save()
} }
// ShouldNotifyOnFailure reports whether the user has enabled desktop // ShouldNotifyOnFailure reports whether the user has enabled desktop
@@ -251,54 +259,75 @@ func (s *Service) UpdateSettings(config domain.Config) error {
config.JobsFile = strings.TrimSpace(config.JobsFile) config.JobsFile = strings.TrimSpace(config.JobsFile)
s.mu.Lock() s.mu.Lock()
jobsPath := storage.ResolveConfiguredPath(s.store.Paths.AppDir, config.JobsFile) // AppDir is fixed for the process and only UpdateSettings itself — a UI
// action — can move JobsPath, so this snapshot stays valid across the reads
// below.
appDir := s.store.Paths.AppDir
jobsPath := storage.ResolveConfiguredPath(appDir, config.JobsFile)
switching := jobsPath != s.store.Paths.JobsPath switching := jobsPath != s.store.Paths.JobsPath
if switching && s.anyRunningLocked() { running := s.anyRunningLocked()
s.mu.Unlock() s.mu.Unlock()
if switching && running {
return errors.New("cannot change the jobs file while a job is running") return errors.New("cannot change the jobs file while a job is running")
} }
// Read the new file before anything is written, so a file that cannot be // Read the new file, and reconstruct its jobs' statistics from the logs the
// parsed leaves both the config and the current jobs untouched. // new config points at, before anything is written and while no lock is held:
// both are file I/O, and SeedStats opens every log in the directory. A file
// that cannot be parsed leaves both the config and the current jobs untouched.
var adopted []domain.Job var adopted []domain.Job
var seeds map[int]runner.SeededStats
if switching { if switching {
jobs, found, err := storage.LoadJobsFile(jobsPath) jobs, found, err := storage.LoadJobsFile(jobsPath)
if err != nil { if err != nil {
s.mu.Unlock()
return fmt.Errorf("read jobs file %s: %w", jobsPath, err) return fmt.Errorf("read jobs file %s: %w", jobsPath, err)
} }
if found { if found {
adopted = jobs adopted = jobs
seeds = runner.SeedStats(storage.ResolveConfiguredPath(appDir, config.LogsDir), jobs, config.MaxLogFiles)
} }
} }
s.store.Config = config s.mu.Lock()
if err := s.store.SaveConfig(); err != nil { // The guard above was evaluated before the reads, off the lock, so re-check
// it: a scheduled run may have started in the meantime, and adoption drops
// every runtime.
if switching && s.anyRunningLocked() {
s.mu.Unlock() s.mu.Unlock()
return err return errors.New("cannot change the jobs file while a job is running")
} }
s.store.Config = config
saveConfig := s.store.PrepareSaveConfig()
if adopted != nil { if adopted != nil {
s.adoptJobsLocked(adopted) s.adoptJobsLocked(adopted)
s.applySeededStatsLocked(seeds)
} }
// SaveConfig re-resolved the paths from the new config, so SaveJobs writes to // PrepareSaveConfig re-resolved the paths from the new config, so the jobs
// the (possibly new) jobs file and cleanup targets the new logs dir. Adopted // write targets the (possibly new) jobs file and cleanup targets the new logs
// jobs are written back too, which persists the IDs and defaults that // dir. Adopted jobs are written back too, which persists the IDs and defaults
// normalization filled in, exactly as loading them at startup would. // that normalization filled in, exactly as loading them at startup would. The
if err := s.store.SaveJobs(s.jobs); err != nil { // jobs write is skipped when the config write fails, because both writes run
s.mu.Unlock() // in the order prepared and stop at the first error.
return err save := s.deferSaveLocked(saveConfig, s.store.PrepareSaveJobs(s.jobs))
}
loaded := len(s.jobs) loaded := len(s.jobs)
logsDir := s.store.Paths.LogsDir logsDir := s.store.Paths.LogsDir
maxFiles := s.store.Config.MaxLogFiles maxFiles := s.store.Config.MaxLogFiles
maxAge := s.store.Config.MaxLogAgeDays maxAge := s.store.Config.MaxLogAgeDays
s.mu.Unlock() s.mu.Unlock()
saveErr := save()
if adopted != nil { if adopted != nil {
// A broad JobChanged redraws the job list; JobsLoaded tells the user in // A broad JobChanged redraws the job list; JobsLoaded tells the user in
// History which file those jobs came from, since nothing was asked. // History which file those jobs came from, since nothing was asked. Both
// are emitted even when the write failed: the adopted jobs are already the
// in-memory list, and a job list the user cannot see would be worse than
// the error they are about to be shown.
s.emit(JobsLoaded{Path: jobsPath, Count: loaded}) s.emit(JobsLoaded{Path: jobsPath, Count: loaded})
s.emit(JobChanged{}) s.emit(JobChanged{})
} }
if saveErr != nil {
return saveErr
}
return runner.CleanupLogs(logsDir, maxFiles, maxAge) return runner.CleanupLogs(logsDir, maxFiles, maxAge)
} }
@@ -474,11 +503,13 @@ func validateConfig(config domain.Config) error {
if strings.TrimSpace(config.LogsDir) == "" { if strings.TrimSpace(config.LogsDir) == "" {
return errors.New("logs directory is required") return errors.New("logs directory is required")
} }
if config.MaxLogFiles <= 0 { // 0 means "keep everything" (see runner.CleanupLogs); only a negative count
return errors.New("max log files must be a positive number") // is rejected, the same three-state shape as DefaultTimeoutSeconds below.
if config.MaxLogFiles < 0 {
return errors.New("max log files must be zero (unlimited) or a positive number")
} }
if config.MaxLogAgeDays <= 0 { if config.MaxLogAgeDays < 0 {
return errors.New("max log age days must be a positive number") return errors.New("max log age days must be zero (unlimited) or a positive number")
} }
if config.ExecutionMode != domain.ExecutionModeParallel && config.ExecutionMode != domain.ExecutionModeSequential { if config.ExecutionMode != domain.ExecutionModeParallel && config.ExecutionMode != domain.ExecutionModeSequential {
return errors.New("execution mode must be 'parallel' or 'sequential'") return errors.New("execution mode must be 'parallel' or 'sequential'")
+119 -8
View File
@@ -3,8 +3,10 @@ package app
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"sync"
"sync/atomic" "sync/atomic"
"testing" "testing"
"time" "time"
@@ -520,9 +522,9 @@ func TestUpdateSettingsPersistsAndValidates(t *testing.T) {
svc := newTempService(t, nil) svc := newTempService(t, nil)
bad := svc.store.Config bad := svc.store.Config
bad.MaxLogFiles = 0 bad.MaxLogFiles = -1
if err := svc.UpdateSettings(bad); err == nil { if err := svc.UpdateSettings(bad); err == nil {
t.Error("expected validation error for non-positive max log files") t.Error("expected validation error for negative max log files")
} }
good := svc.store.Config good := svc.store.Config
@@ -531,8 +533,21 @@ func TestUpdateSettingsPersistsAndValidates(t *testing.T) {
if err := svc.UpdateSettings(good); err != nil { if err := svc.UpdateSettings(good); err != nil {
t.Fatalf("UpdateSettings: %v", err) t.Fatalf("UpdateSettings: %v", err)
} }
if svc.Store().Config.MaxLogAgeDays != 7 || svc.Store().Config.NotifyOnFailure { if svc.store.Config.MaxLogAgeDays != 7 || svc.store.Config.NotifyOnFailure {
t.Errorf("config not applied: %+v", svc.Store().Config) t.Errorf("config not applied: %+v", svc.store.Config)
}
// 0 means "keep everything" (see STANDARDS §Intentional behavior), not an
// invalid value, so it must be accepted and persisted rather than rejected
// or silently backfilled.
unlimited := svc.store.Config
unlimited.MaxLogFiles = 0
unlimited.MaxLogAgeDays = 0
if err := svc.UpdateSettings(unlimited); err != nil {
t.Fatalf("UpdateSettings with zero retention limits: %v", err)
}
if svc.store.Config.MaxLogFiles != 0 || svc.store.Config.MaxLogAgeDays != 0 {
t.Errorf("zero retention limits not preserved: %+v", svc.store.Config)
} }
} }
@@ -547,8 +562,8 @@ func TestUpdateSettingsRejectsInvalidConfigs(t *testing.T) {
{"missing jobs file", func(c *domain.Config) { c.JobsFile = " " }}, {"missing jobs file", func(c *domain.Config) { c.JobsFile = " " }},
{"jobs file without a file name", func(c *domain.Config) { c.JobsFile = "jobs" + string(filepath.Separator) }}, {"jobs file without a file name", func(c *domain.Config) { c.JobsFile = "jobs" + string(filepath.Separator) }},
{"missing logs dir", func(c *domain.Config) { c.LogsDir = "" }}, {"missing logs dir", func(c *domain.Config) { c.LogsDir = "" }},
{"non-positive max files", func(c *domain.Config) { c.MaxLogFiles = 0 }}, {"negative max files", func(c *domain.Config) { c.MaxLogFiles = -1 }},
{"non-positive max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }}, {"negative max age", func(c *domain.Config) { c.MaxLogAgeDays = -1 }},
{"negative default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = -1 }}, {"negative default timeout", func(c *domain.Config) { c.DefaultTimeoutSeconds = -1 }},
} }
for _, tc := range tests { for _, tc := range tests {
@@ -710,12 +725,12 @@ func TestUpdateSettingsRefusesJobsFileSwitchWhileRunning(t *testing.T) {
if err := svc.UpdateSettings(config); err == nil { if err := svc.UpdateSettings(config); err == nil {
t.Error("expected the jobs-file switch to be refused while a job is running") t.Error("expected the jobs-file switch to be refused while a job is running")
} }
if svc.Store().Config.JobsFile == config.JobsFile { if svc.store.Config.JobsFile == config.JobsFile {
t.Error("the refused switch must not have been persisted") t.Error("the refused switch must not have been persisted")
} }
// A setting that does not touch the jobs file still saves during a run. // A setting that does not touch the jobs file still saves during a run.
unrelated := svc.Store().Config unrelated := svc.store.Config
unrelated.NotifyOnFailure = !unrelated.NotifyOnFailure unrelated.NotifyOnFailure = !unrelated.NotifyOnFailure
if err := svc.UpdateSettings(unrelated); err != nil { if err := svc.UpdateSettings(unrelated); err != nil {
t.Errorf("unrelated setting should still save during a run: %v", err) t.Errorf("unrelated setting should still save during a run: %v", err)
@@ -725,6 +740,102 @@ func TestUpdateSettingsRefusesJobsFileSwitchWhileRunning(t *testing.T) {
waitRecord(t, done) waitRecord(t, done)
} }
// Adoption reconstructs the adopted jobs' aggregate statistics from the log
// files the new configuration points at. That scan opens every log in the
// directory, so UpdateSettings runs it before taking the state lock; this pins
// that its result still reaches the runtime map.
func TestUpdateSettingsSeedsAdoptedJobsFromLogs(t *testing.T) {
svc := newTempService(t, []domain.Job{{ID: 1, Name: "Local", Schedule: "@every 1m", Command: "echo local", Enabled: true}})
logsDir := svc.store.Paths.LogsDir
if err := os.MkdirAll(logsDir, 0o755); err != nil {
t.Fatal(err)
}
log := "time: 2026-08-05 10:00:00\njob_id: 7\njob_name: Adopted\ntrigger: Schedule\nstate: Failed\ndetail: boom\nduration: 1500\n\nstdout:\n<empty>\n"
if err := os.WriteFile(filepath.Join(logsDir, "20260805-100000_Adopted.log"), []byte(log), 0o644); err != nil {
t.Fatal(err)
}
shared := filepath.Join(svc.store.Paths.AppDir, "shared.json")
data, err := json.Marshal(domain.JobsFile{Jobs: []domain.Job{
{ID: 7, Name: "Adopted", Schedule: "@every 5m", Command: "echo adopted", Enabled: true},
}})
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(shared, data, 0o644); err != nil {
t.Fatal(err)
}
config := svc.store.Config
config.JobsFile = shared
if err := svc.UpdateSettings(config); err != nil {
t.Fatalf("UpdateSettings: %v", err)
}
runtime := svc.Runtime(7)
if runtime == nil {
t.Fatal("the adopted job has no runtime")
}
if runtime.RunCount != 1 || runtime.FailCount != 1 || runtime.LastDurationMS != 1500 {
t.Errorf("seeded stats: RunCount=%d FailCount=%d LastDurationMS=%d, want 1/1/1500",
runtime.RunCount, runtime.FailCount, runtime.LastDurationMS)
}
}
// Job saves run after mu is released, so one operation can be writing while
// another mutates state. deferSaveLocked takes its own lock while mu is still
// held, which is what keeps writes in mutation order: whatever changed the list
// last also wrote it last, so the file ends up matching memory instead of
// holding an older snapshot.
func TestConcurrentJobOperationsLeaveTheFileMatchingMemory(t *testing.T) {
svc := newTempService(t, nil)
const workers = 8
var wg sync.WaitGroup
for i := 0; i < workers; i++ {
wg.Add(1)
go func(i int) {
defer wg.Done()
job, err := svc.CreateJob(domain.Job{Name: fmt.Sprintf("Job %d", i), Schedule: "@every 1m", Command: "echo hi", Enabled: true})
if err != nil {
t.Errorf("CreateJob %d: %v", i, err)
return
}
if err := svc.SetEnabled(job.ID, false); err != nil {
t.Errorf("SetEnabled %d: %v", job.ID, err)
}
}(i)
}
wg.Wait()
memory := svc.Jobs()
if len(memory) != workers {
t.Fatalf("jobs in memory = %d, want %d", len(memory), workers)
}
saved, found, err := storage.LoadJobsFile(svc.store.Paths.JobsPath)
if err != nil || !found {
t.Fatalf("read jobs file: found=%v err=%v", found, err)
}
if len(saved) != len(memory) {
t.Fatalf("jobs on disk = %d, want %d: the last write must be the last mutation", len(saved), len(memory))
}
onDisk := make(map[int]domain.Job, len(saved))
for _, job := range saved {
onDisk[job.ID] = job
}
for _, job := range memory {
got, ok := onDisk[job.ID]
if !ok {
t.Errorf("job %d (%q) is in memory but missing from the file", job.ID, job.Name)
continue
}
if got.Name != job.Name || got.Enabled != job.Enabled {
t.Errorf("job %d on disk = %q/%v, want %q/%v", job.ID, got.Name, got.Enabled, job.Name, job.Enabled)
}
}
}
func TestPrependLogCapsActivityList(t *testing.T) { func TestPrependLogCapsActivityList(t *testing.T) {
runtime := &domain.JobRuntime{} runtime := &domain.JobRuntime{}
for i := 0; i < maxJobLogs+10; i++ { for i := 0; i < maxJobLogs+10; i++ {
+10 -3
View File
@@ -1,19 +1,26 @@
package app package app
import ( import (
"fmt"
"gitea.mixdep.ru/mix/gosentry/src/platform/desktop" "gitea.mixdep.ru/mix/gosentry/src/platform/desktop"
) )
// InstallDesktopIcon installs the application's .desktop file and icon on // InstallDesktopIcon installs the application's .desktop file and icon on
// Linux (no-op on other platforms). The resulting icon path is stored in // Linux (no-op on other platforms). The resulting icon path is stored in
// store.Paths.DesktopIcon so ApplyAutostart can reference it. // store.Paths.DesktopIcon so ApplyAutostart can reference it. A failure is
// reported through ErrorOccurred rather than discarded, so the visible symptom
// (a generic dock icon) has an explanation in History instead of none.
func (s *Service) InstallDesktopIcon(appID string, iconBytes []byte) { func (s *Service) InstallDesktopIcon(appID string, iconBytes []byte) {
if iconPath, err := desktop.InstallDesktopIntegration(appID, s.store.Paths.ExecutablePath, iconBytes); err == nil { iconPath, err := desktop.InstallDesktopIntegration(appID, s.store.Paths.ExecutablePath, iconBytes)
if err != nil {
s.emit(ErrorOccurred{Err: fmt.Errorf("install desktop icon: %w", err)})
return
}
s.mu.Lock() s.mu.Lock()
s.store.Paths.DesktopIcon = iconPath s.store.Paths.DesktopIcon = iconPath
s.mu.Unlock() s.mu.Unlock()
} }
}
// AutostartStatus reports whether the platform autostart entry matches the // AutostartStatus reports whether the platform autostart entry matches the
// current StartOnLogin and KeepRunningInTray settings in the stored config. // current StartOnLogin and KeepRunningInTray settings in the stored config.
+8 -3
View File
@@ -147,7 +147,6 @@ func (s *Service) executeRun(ctx context.Context, jobCopy domain.Job, trigger st
record, logErr := s.runJob(ctx, &jobCopy, trigger, env.logsDir, env.timeout) record, logErr := s.runJob(ctx, &jobCopy, trigger, env.logsDir, env.timeout)
s.mu.Lock() s.mu.Lock()
var cleanupErr error
var rerunStarted bool var rerunStarted bool
if current := s.findByIDLocked(jobCopy.ID); current != nil { if current := s.findByIDLocked(jobCopy.ID); current != nil {
runtime := s.runtimeForLocked(current) runtime := s.runtimeForLocked(current)
@@ -166,10 +165,15 @@ func (s *Service) executeRun(ctx context.Context, jobCopy domain.Job, trigger st
} else { } else {
s.refreshNextRunLocked(current, runtime) s.refreshNextRunLocked(current, runtime)
} }
cleanupErr = runner.CleanupLogs(env.logsDir, env.maxFiles, env.maxAge)
} }
s.mu.Unlock() s.mu.Unlock()
// Cleanup is a directory scan plus up to MaxLogFiles unlinks. It needs only
// the values already snapshotted into runEnv, so it runs after mu is released
// rather than making every UI refresh wait behind it. It runs even when the
// job is gone, because the run still wrote a log file that retention covers.
cleanupErr := runner.CleanupLogs(env.logsDir, env.maxFiles, env.maxAge)
if logErr != nil { if logErr != nil {
s.emit(ErrorOccurred{Err: fmt.Errorf("write run log for %q: %w", jobCopy.Name, logErr)}) s.emit(ErrorOccurred{Err: fmt.Errorf("write run log for %q: %w", jobCopy.Name, logErr)})
} }
@@ -249,7 +253,8 @@ func updateStats(rt *domain.JobRuntime, r domain.RunRecord) {
rt.MaxDurationMS = r.DurationMS rt.MaxDurationMS = r.DurationMS
} }
rt.TimedRunCount++ rt.TimedRunCount++
rt.AvgDurationMS = (rt.AvgDurationMS*int64(rt.TimedRunCount-1) + r.DurationMS) / int64(rt.TimedRunCount) rt.DurationSumMS += r.DurationMS
rt.AvgDurationMS = rt.DurationSumMS / int64(rt.TimedRunCount)
} }
// runningOutput is the placeholder output shown while a job is running, before // runningOutput is the placeholder output shown while a job is running, before
+9
View File
@@ -103,6 +103,15 @@ func TestUpdateStats(t *testing.T) {
if rt.AvgDurationMS != 233 { if rt.AvgDurationMS != 233 {
t.Errorf("after run 3: avg=%d, want 233", rt.AvgDurationMS) t.Errorf("after run 3: avg=%d, want 233", rt.AvgDurationMS)
} }
// AvgDurationMS must always be exactly DurationSumMS/TimedRunCount — a stored
// sum divided once, not an incremental mean that truncates on every step and
// compounds error over a long-running job.
if rt.DurationSumMS != 700 {
t.Errorf("DurationSumMS = %d, want 700", rt.DurationSumMS)
}
if want := rt.DurationSumMS / int64(rt.TimedRunCount); rt.AvgDurationMS != want {
t.Errorf("AvgDurationMS = %d, want DurationSumMS/TimedRunCount = %d", rt.AvgDurationMS, want)
}
} }
func TestUpdateStatsSkipsZeroDuration(t *testing.T) { func TestUpdateStatsSkipsZeroDuration(t *testing.T) {
+70 -13
View File
@@ -26,7 +26,10 @@ import (
// it; unexported helpers ending in "Locked" assume the caller already holds it. // it; unexported helpers ending in "Locked" assume the caller already holds it.
// The Service must never call back into the UI (or any code that might re-enter // The Service must never call back into the UI (or any code that might re-enter
// the Service) while holding mu — in particular emit() is always called after // the Service) while holding mu — in particular emit() is always called after
// mu is released. // mu is released. Blocking file I/O follows the same rule: mu is the lock the
// Fyne main thread takes on every Jobs() and Runtime() call, so a JSON write, a
// log-directory scan, or a pass over every log header must not happen inside it
// (see deferSaveLocked, executeRun, and applySeededStatsLocked).
type Service struct { type Service struct {
mu sync.Mutex mu sync.Mutex
store *storage.Store store *storage.Store
@@ -56,6 +59,13 @@ type Service struct {
// do not exercise autostart; Open() wires it via autostart.New(). // do not exercise autostart; Open() wires it via autostart.New().
manager autostart.Manager manager autostart.Manager
// saveMu serializes the store writes that operations prepare under mu and run
// after releasing it. It is taken while mu is still held and released once the
// write is done, so writes reach the file in the same order their snapshots
// were taken and an older snapshot can never land on top of a newer one.
// Nothing may take mu while holding saveMu.
saveMu sync.Mutex
// observers and their guard live in events.go. dispatchMu is separate from mu // observers and their guard live in events.go. dispatchMu is separate from mu
// so that emitting an event never requires (or is held under) the state lock: // so that emitting an event never requires (or is held under) the state lock:
// the Service must release mu before dispatching, per the locking contract. // the Service must release mu before dispatching, per the locking contract.
@@ -63,6 +73,26 @@ type Service struct {
observers []Observer observers []Observer
} }
// deferSaveLocked prepares the store writes for the caller to run after mu is
// released, and takes saveMu now so a later operation's write cannot overtake
// this one. The caller must hold mu, must unlock it before calling the returned
// function, and must call that function exactly once. Keeping the marshal, the
// fsync, and the rename out of the critical section is what stops a settings
// change or a job edit from blocking a scheduler tick or a finishing run. The
// writes run in the order given and stop at the first error.
func (s *Service) deferSaveLocked(writes ...func() error) func() error {
s.saveMu.Lock()
return func() error {
defer s.saveMu.Unlock()
for _, write := range writes {
if err := write(); err != nil {
return err
}
}
return nil
}
}
// NewService wires the Service to a loaded store and its jobs. It builds the // NewService wires the Service to a loaded store and its jobs. It builds the
// initial runtime map from the durable jobs so every job has transient state // initial runtime map from the durable jobs so every job has transient state
// from the moment the Service exists, and parses each job's schedule once. The // from the moment the Service exists, and parses each job's schedule once. The
@@ -77,18 +107,19 @@ func NewService(store *storage.Store, jobs []domain.Job) *Service {
// No lock is needed here: construction is single-threaded, before Start // No lock is needed here: construction is single-threaded, before Start
// launches the timing loop. // launches the timing loop.
s.adoptJobsLocked(jobs) s.adoptJobsLocked(jobs)
s.applySeededStatsLocked(runner.SeedStats(store.Paths.LogsDir, s.jobs, store.Config.MaxLogFiles))
return s return s
} }
// adoptJobsLocked makes jobs the Service's durable state and rebuilds everything // adoptJobsLocked makes jobs the Service's durable state and rebuilds everything
// derived from it: the runtime map, the parsed-schedule cache, each job's first // derived from it: the runtime map, the parsed-schedule cache, and each job's
// next-run — so the Service is ready to schedule the moment it exists, mirroring // first next-run — so the Service is ready to schedule the moment it exists,
// the old scheduler's reset-on-construction — and the statistics seeded from // mirroring the old scheduler's reset-on-construction.
// existing log files, so the details panel shows accumulated run history
// immediately rather than only runs since this process started.
// //
// It backs both construction and a Settings change that points at a different // It backs both construction and a Settings change that points at a different
// jobs file. The caller must hold mu. // jobs file. Statistics seeded from existing log files are applied separately by
// applySeededStatsLocked, because reconstructing them is file I/O. The caller
// must hold mu.
func (s *Service) adoptJobsLocked(jobs []domain.Job) { func (s *Service) adoptJobsLocked(jobs []domain.Job) {
s.jobs = jobs s.jobs = jobs
s.runtimes = domain.NewRuntimes(jobs) s.runtimes = domain.NewRuntimes(jobs)
@@ -100,7 +131,16 @@ func (s *Service) adoptJobsLocked(jobs []domain.Job) {
s.parseScheduleLocked(job) s.parseScheduleLocked(job)
s.refreshNextRunFromLocked(job, s.runtimes[job.ID], now) s.refreshNextRunFromLocked(job, s.runtimes[job.ID], now)
} }
for id, seed := range runner.SeedStats(s.store.Paths.LogsDir, s.jobs, s.store.Config.MaxLogFiles) { }
// applySeededStatsLocked folds statistics reconstructed from existing log files
// into the runtime map, so the details panel shows accumulated run history
// immediately rather than only runs since this process started. It is separate
// from adoptJobsLocked because producing the seeds opens every log file in the
// directory, which must not happen under mu: callers compute the map first and
// apply it here. The caller must hold mu.
func (s *Service) applySeededStatsLocked(seeds map[int]runner.SeededStats) {
for id, seed := range seeds {
runtime := s.runtimes[id] runtime := s.runtimes[id]
if runtime == nil { if runtime == nil {
continue continue
@@ -111,6 +151,7 @@ func (s *Service) adoptJobsLocked(jobs []domain.Job) {
runtime.AvgDurationMS = seed.AvgDurationMS runtime.AvgDurationMS = seed.AvgDurationMS
runtime.MaxDurationMS = seed.MaxDurationMS runtime.MaxDurationMS = seed.MaxDurationMS
runtime.TimedRunCount = seed.TimedRunCount runtime.TimedRunCount = seed.TimedRunCount
runtime.DurationSumMS = seed.DurationSumMS
} }
} }
@@ -164,11 +205,27 @@ func Open() (*Service, error) {
return svc, nil return svc, nil
} }
// Store returns the underlying store. It is exposed so callers that still need // Config returns a copy of the current application configuration, safe to
// resolved paths and config (the GUI, during the transition) can reach them; // call from any goroutine. UpdateSettings, SetGlobalPause, and SetJobListView
// later phases narrow this surface. // are the only writers and all mutate store.Config under mu; copying under the
func (s *Service) Store() *storage.Store { // same lock is what keeps a UI read from racing them, instead of holding onto
return s.store // the *storage.Store this used to hand out (see STANDARDS: the UI reads
// Service state through typed events and accessors, never shared mutable
// state).
func (s *Service) Config() domain.Config {
s.mu.Lock()
defer s.mu.Unlock()
return s.store.Config
}
// Paths returns a copy of the store's resolved filesystem paths. AppDir and
// ConfigPath are fixed for the process; JobsPath, JobsDir, and LogsDir are
// re-derived under mu on every settings save (storage.Store.applyConfigPaths),
// so this copies under the same lock as Config for the same reason.
func (s *Service) Paths() storage.Paths {
s.mu.Lock()
defer s.mu.Unlock()
return s.store.Paths
} }
// Jobs returns a copy of the durable jobs slice. Returning a copy keeps callers // Jobs returns a copy of the durable jobs slice. Returning a copy keeps callers
+1 -1
View File
@@ -3,4 +3,4 @@ package app
// Version is the application version shown in the GUI and used by build // Version is the application version shown in the GUI and used by build
// scripts in artifact names. It is a var rather than a const so release builds // scripts in artifact names. It is a var rather than a const so release builds
// can override it with Go ldflags when CI tags a build. // can override it with Go ldflags when CI tags a build.
var Version = "1.0.2" var Version = "1.0.3"
+9 -9
View File
@@ -4,13 +4,13 @@ package domain
// output is also written to a log file; the in-memory Output copy exists so the // output is also written to a log file; the in-memory Output copy exists so the
// latest run can be displayed without reopening the log on every repaint. // latest run can be displayed without reopening the log on every repaint.
type RunRecord struct { type RunRecord struct {
Time string `yaml:"time"` Time string
JobID int `yaml:"job_id"` JobID int
JobName string `yaml:"job_name"` JobName string
Trigger string `yaml:"trigger,omitempty"` Trigger string
State string `yaml:"state"` State string
Detail string `yaml:"detail"` Detail string
LogFile string `yaml:"log_file,omitempty"` LogFile string
Output string `yaml:"output,omitempty"` Output string
DurationMS int64 `yaml:"duration_ms,omitempty"` DurationMS int64
} }
+8
View File
@@ -36,6 +36,14 @@ type JobRuntime struct {
// launches that round to 0) increment RunCount but not this. StartOnly runs // launches that round to 0) increment RunCount but not this. StartOnly runs
// otherwise contribute their launch latency. // otherwise contribute their launch latency.
TimedRunCount int TimedRunCount int
// DurationSumMS is the running total of every timed run's duration.
// AvgDurationMS is always DurationSumMS/TimedRunCount, computed fresh on each
// update rather than folded incrementally — an incremental integer mean
// truncates on every step, and the error compounds over the life of a job
// that keeps running. A stored sum divided once per update matches the exact
// sum/count average runner.aggregateLogStats computes when seeding from logs,
// so the two no longer disagree about the same run history.
DurationSumMS int64
} }
// NewRuntime builds the initial runtime state for a freshly loaded or created // NewRuntime builds the initial runtime state for a freshly loaded or created
+4 -4
View File
@@ -18,16 +18,16 @@ type linuxManager struct{}
func New() Manager { return linuxManager{} } func New() Manager { return linuxManager{} }
func (linuxManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { func (linuxManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, startInTray, executablePath, iconPath) return setAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (linuxManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { func (linuxManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, startInTray, executablePath) return autostartStatus(expectedEnabled, startInTray, executablePath)
} }
const autostartDesktopFileName = "gosentry.desktop" const autostartDesktopFileName = "gosentry.desktop"
func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error {
desktopPath, err := autostartDesktopPath() desktopPath, err := autostartDesktopPath()
if err != nil { if err != nil {
return err return err
@@ -58,7 +58,7 @@ X-GNOME-Autostart-enabled=true
return nil return nil
} }
func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) {
desktopPath, err := autostartDesktopPath() desktopPath, err := autostartDesktopPath()
if err != nil { if err != nil {
return false, "Cannot resolve XDG autostart directory" return false, "Cannot resolve XDG autostart directory"
@@ -14,7 +14,7 @@ func TestLinuxAutostartStartsInTray(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("XDG_CONFIG_HOME", t.TempDir())
executablePath := "/opt/Go Sentry/gosentry" executablePath := "/opt/Go Sentry/gosentry"
if err := SetAutostart(true, true, executablePath, "/opt/Go Sentry/gosentry.png"); err != nil { if err := setAutostart(true, true, executablePath, "/opt/Go Sentry/gosentry.png"); err != nil {
t.Fatalf("enable autostart: %v", err) t.Fatalf("enable autostart: %v", err)
} }
@@ -37,7 +37,7 @@ func TestLinuxAutostartWithoutTrayFlag(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("XDG_CONFIG_HOME", t.TempDir())
executablePath := "/opt/Go Sentry/gosentry" executablePath := "/opt/Go Sentry/gosentry"
if err := SetAutostart(true, false, executablePath, ""); err != nil { if err := setAutostart(true, false, executablePath, ""); err != nil {
t.Fatalf("enable autostart: %v", err) t.Fatalf("enable autostart: %v", err)
} }
+4 -4
View File
@@ -10,21 +10,21 @@ type otherManager struct{}
func New() Manager { return otherManager{} } func New() Manager { return otherManager{} }
func (otherManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { func (otherManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, startInTray, executablePath, iconPath) return setAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (otherManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { func (otherManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, startInTray, executablePath) return autostartStatus(expectedEnabled, startInTray, executablePath)
} }
func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error {
if !enabled { if !enabled {
return nil return nil
} }
return fmt.Errorf("autostart is not implemented for this platform") return fmt.Errorf("autostart is not implemented for this platform")
} }
func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) {
if !expectedEnabled { if !expectedEnabled {
return true, "Autostart is off" return true, "Autostart is off"
} }
+5 -5
View File
@@ -17,17 +17,17 @@ type windowsManager struct{}
func New() Manager { return windowsManager{} } func New() Manager { return windowsManager{} }
func (windowsManager) Set(enabled, startInTray bool, executablePath, iconPath string) error { func (windowsManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, startInTray, executablePath, iconPath) return setAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (windowsManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) { func (windowsManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, startInTray, executablePath) return autostartStatus(expectedEnabled, startInTray, executablePath)
} }
const autostartName = "GoSentry" const autostartName = "GoSentry"
const startupShortcutFile = autostartName + ".lnk" const startupShortcutFile = autostartName + ".lnk"
func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error { func setAutostart(enabled bool, startInTray bool, executablePath string, iconPath string) error {
// Windows autostart used to write HKCU\Run values, but that approach became // Windows autostart used to write HKCU\Run values, but that approach became
// brittle once paths with spaces and the "--start-in-tray" argument entered // brittle once paths with spaces and the "--start-in-tray" argument entered
// the picture. A Startup-folder shortcut stores target path and arguments as // the picture. A Startup-folder shortcut stores target path and arguments as
@@ -44,7 +44,7 @@ func SetAutostart(enabled bool, startInTray bool, executablePath string, iconPat
return removeIfExists(shortcutPath) return removeIfExists(shortcutPath)
} }
func AutostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) { func autostartStatus(expectedEnabled bool, startInTray bool, executablePath string) (bool, string) {
shortcutPath, err := startupShortcutPath() shortcutPath, err := startupShortcutPath()
if err != nil { if err != nil {
return false, "Startup folder cannot be resolved" return false, "Startup folder cannot be resolved"
@@ -126,7 +126,7 @@ func readShortcut(shortcutPath string) (string, string, error) {
// OEM code page (e.g. CP866 on Russian Windows). Without this override, // OEM code page (e.g. CP866 on Russian Windows). Without this override,
// [Console]::Out.Write encodes Cyrillic and other non-ASCII characters as // [Console]::Out.Write encodes Cyrillic and other non-ASCII characters as
// OEM bytes; Go then reads them as UTF-8 and gets a different string from // OEM bytes; Go then reads them as UTF-8 and gets a different string from
// os.Executable, causing AutostartStatus to report "shortcut points to // os.Executable, causing autostartStatus to report "shortcut points to
// another executable" for any install path that contains non-ASCII chars. // another executable" for any install path that contains non-ASCII chars.
// New-Object System.Text.UTF8Encoding($false) is UTF-8 without BOM. // New-Object System.Text.UTF8Encoding($false) is UTF-8 without BOM.
script := `[Console]::OutputEncoding = New-Object System.Text.UTF8Encoding($false); $shell = New-Object -ComObject WScript.Shell; $shortcut = $shell.CreateShortcut($env:GOSENTRY_SHORTCUT_PATH); [Console]::Out.Write($shortcut.TargetPath + [Environment]::NewLine + $shortcut.Arguments)` script := `[Console]::OutputEncoding = New-Object System.Text.UTF8Encoding($false); $shell = New-Object -ComObject WScript.Shell; $shortcut = $shell.CreateShortcut($env:GOSENTRY_SHORTCUT_PATH); [Console]::Out.Write($shortcut.TargetPath + [Environment]::NewLine + $shortcut.Arguments)`
@@ -134,7 +134,7 @@ func TestAutostartStatusRequiresMatchingTrayFlag(t *testing.T) {
t.Fatalf("create shortcut: %v", err) t.Fatalf("create shortcut: %v", err)
} }
ok, message := AutostartStatus(true, false, targetPath) ok, message := autostartStatus(true, false, targetPath)
if ok { if ok {
t.Fatalf("expected problem when tray flag mismatches, got OK: %s", message) t.Fatalf("expected problem when tray flag mismatches, got OK: %s", message)
} }
+5
View File
@@ -9,6 +9,11 @@ import (
"time" "time"
) )
// CleanupLogs enforces the count and age retention policies on the .log files
// in logsDir. maxFiles <= 0 disables the count policy and maxAgeDays <= 0
// disables the age policy, independently — "keep everything" is a value the
// user can choose in Settings, not just an internal default (STANDARDS
// §Intentional behavior).
func CleanupLogs(logsDir string, maxFiles int, maxAgeDays int) error { func CleanupLogs(logsDir string, maxFiles int, maxAgeDays int) error {
entries, err := os.ReadDir(logsDir) entries, err := os.ReadDir(logsDir)
if err != nil { if err != nil {
-2
View File
@@ -64,5 +64,3 @@ func LogArguments(arguments string) string {
} }
return strings.ReplaceAll(strings.TrimSpace(arguments), "\r\n", "\n") return strings.ReplaceAll(strings.TrimSpace(arguments), "\r\n", "\n")
} }
func logArguments(arguments string) string { return LogArguments(arguments) }
+22 -2
View File
@@ -23,9 +23,9 @@ func writeRunLog(logsDir string, job domain.Job, trigger string, state string, d
// by run time. The job name is included for human scanning, but sanitized to // by run time. The job name is included for human scanning, but sanitized to
// avoid characters that are invalid on Windows or awkward on shells. // avoid characters that are invalid on Windows or awkward on shells.
fileName := started.Format("20060102-150405") + "_" + sanitizeFileName(job.Name) + ".log" fileName := started.Format("20060102-150405") + "_" + sanitizeFileName(job.Name) + ".log"
path := filepath.Join(logsDir, fileName) path := uniqueLogPath(logsDir, fileName)
content := fmt.Sprintf("time: %s\njob_id: %d\njob_name: %s\ntrigger: %s\nstate: %s\ndetail: %s\nduration: %d\ncommand: %s\narguments: %s\nstart_only: %t\n\n%s\n", content := fmt.Sprintf("time: %s\njob_id: %d\njob_name: %s\ntrigger: %s\nstate: %s\ndetail: %s\nduration: %d\ncommand: %s\narguments: %s\nstart_only: %t\n\n%s\n",
started.Format("2006-01-02 15:04:05"), job.ID, job.Name, trigger, state, detail, durationMS, job.Command, logArguments(job.Arguments), job.StartOnly, output) started.Format("2006-01-02 15:04:05"), job.ID, job.Name, trigger, state, detail, durationMS, job.Command, LogArguments(job.Arguments), job.StartOnly, output)
if err := writeFileAtomic(logsDir, path, []byte(content), 0o644); err != nil { if err := writeFileAtomic(logsDir, path, []byte(content), 0o644); err != nil {
return "", fmt.Errorf("write log file: %w", err) return "", fmt.Errorf("write log file: %w", err)
} }
@@ -70,6 +70,26 @@ func writeFileAtomic(dir, path string, data []byte, perm os.FileMode) error {
return nil return nil
} }
// uniqueLogPath returns a path for fileName in dir, appending a disambiguating
// "-2", "-3", … suffix before the extension if the plain name is already
// taken. Two runs of the same job in the same second — a fast manual re-run,
// or a sub-second queue drain — would otherwise share one timestamp and the
// second write would silently overwrite the first.
func uniqueLogPath(dir, fileName string) string {
path := filepath.Join(dir, fileName)
if _, err := os.Stat(path); err != nil {
return path
}
ext := filepath.Ext(fileName)
base := strings.TrimSuffix(fileName, ext)
for n := 2; ; n++ {
candidate := filepath.Join(dir, fmt.Sprintf("%s-%d%s", base, n, ext))
if _, err := os.Stat(candidate); err != nil {
return candidate
}
}
}
func sanitizeFileName(name string) string { func sanitizeFileName(name string) string {
name = strings.TrimSpace(name) name = strings.TrimSpace(name)
if name == "" { if name == "" {
+36
View File
@@ -0,0 +1,36 @@
package runner
import (
"os"
"path/filepath"
"testing"
)
// TestUniqueLogPathAvoidsCollision pins the fix for two runs of the same job
// landing on the same second: without disambiguation the second write would
// silently overwrite the first.
func TestUniqueLogPathAvoidsCollision(t *testing.T) {
dir := t.TempDir()
const name = "20260101-120000_job.log"
first := uniqueLogPath(dir, name)
if first != filepath.Join(dir, name) {
t.Fatalf("first call: got %q, want the plain name", first)
}
if err := os.WriteFile(first, []byte("one"), 0o644); err != nil {
t.Fatal(err)
}
second := uniqueLogPath(dir, name)
if second == first {
t.Fatalf("second call returned the same path as an existing file: %q", second)
}
if err := os.WriteFile(second, []byte("two"), 0o644); err != nil {
t.Fatal(err)
}
third := uniqueLogPath(dir, name)
if third == first || third == second {
t.Fatalf("third call collided with an existing file: %q (existing: %q, %q)", third, first, second)
}
}
+10 -2
View File
@@ -36,7 +36,15 @@ func RunJob(ctx context.Context, job *domain.Job, trigger string, logsDir string
var detail string var detail string
var durationMS int64 var durationMS int64
if job.StartOnly { if job.StartOnly {
invocation := jobInvocation(ctx, *job) // A StartOnly process is deliberately never waited for, so it must not be
// tied to any cancelable context: exec.CommandContext leaves a watcher
// goroutine alive until Wait returns or the context is done, and since
// StartOnly never calls Wait that goroutine would live for the rest of the
// process — one per run — and then try to kill a process whose handle
// startJobOnly has already released. context.Background() has a nil Done
// channel, so os/exec starts no watcher at all and the started process is
// left to outlive GoSentry, which is the point of the option.
invocation := jobInvocation(context.Background(), *job)
// StartOnly jobs don't wait for process exit, so the duration measures // StartOnly jobs don't wait for process exit, so the duration measures
// launch latency (time to spawn the process) rather than run time. // launch latency (time to spawn the process) rather than run time.
state, detail, output, durationMS = startJobOnly(invocation, *job, started) state, detail, output, durationMS = startJobOnly(invocation, *job, started)
@@ -108,7 +116,7 @@ func startOnlyOutput(job domain.Job, pid int) string {
builder.WriteString("command:\n") builder.WriteString("command:\n")
builder.WriteString(job.Command + "\n\n") builder.WriteString(job.Command + "\n\n")
builder.WriteString("arguments:\n") builder.WriteString("arguments:\n")
builder.WriteString(logArguments(job.Arguments)) builder.WriteString(LogArguments(job.Arguments))
builder.WriteString("\n\nstart_only:\ntrue") builder.WriteString("\n\nstart_only:\ntrue")
return builder.String() return builder.String()
} }
+54 -2
View File
@@ -135,8 +135,8 @@ func TestLogArguments(t *testing.T) {
{"--flag\n--value", "--flag\n--value"}, {"--flag\n--value", "--flag\n--value"},
} }
for _, tc := range cases { for _, tc := range cases {
if got := logArguments(tc.input); got != tc.want { if got := LogArguments(tc.input); got != tc.want {
t.Errorf("logArguments(%q) = %q, want %q", tc.input, got, tc.want) t.Errorf("LogArguments(%q) = %q, want %q", tc.input, got, tc.want)
} }
} }
} }
@@ -401,6 +401,58 @@ func TestRunJobZeroTimeoutMeansNoTimeout(t *testing.T) {
} }
} }
// A StartOnly run must not leave a watcher goroutine behind. exec.CommandContext
// keeps one alive until Wait returns or the context is done, and StartOnly never
// waits, so binding it to the caller's cancelable context would leak one
// goroutine per run for the lifetime of the app — and then, on shutdown, kill a
// process whose handle startJobOnly has already released.
func TestRunJobStartOnlyLeavesNoContextWatcher(t *testing.T) {
command := "sh"
arguments := "-c\nexit 0"
if runtime.GOOS == "windows" {
command = `C:\Windows\System32\cmd.exe`
arguments = "/C\nexit /b 0"
}
job := domain.Job{
ID: 53,
Name: "Start Only Goroutines",
Command: command,
Arguments: arguments,
StartOnly: true,
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
const runs = 5
before := settledGoroutines()
for i := 0; i < runs; i++ {
if _, err := RunJob(ctx, &job, "Manual", t.TempDir(), 30*time.Second); err != nil {
t.Fatal(err)
}
}
// Counted before cancel on purpose: a watcher would still be parked on
// ctx.Done() at this point, and cancelling first would release it.
if leaked := settledGoroutines() - before; leaked > 1 {
t.Errorf("%d goroutines left after %d StartOnly runs, want none tied to the run context", leaked, runs)
}
}
// settledGoroutines returns the goroutine count once it has stopped falling, so
// a goroutine that is still on its way out is not mistaken for a leak.
func settledGoroutines() int {
lowest := runtime.NumGoroutine()
for stable, i := 0, 0; stable < 3 && i < 100; i++ {
time.Sleep(10 * time.Millisecond)
if count := runtime.NumGoroutine(); count < lowest {
lowest, stable = count, 0
continue
}
stable++
}
return lowest
}
func TestRunJobStartOnlyIgnoresTimeout(t *testing.T) { func TestRunJobStartOnlyIgnoresTimeout(t *testing.T) {
command := "sh" command := "sh"
arguments := "-c\nsleep 5" arguments := "-c\nsleep 5"
+56 -55
View File
@@ -21,6 +21,10 @@ type SeededStats struct {
AvgDurationMS int64 AvgDurationMS int64
MaxDurationMS int64 MaxDurationMS int64
TimedRunCount int TimedRunCount int
// DurationSumMS is the running total AvgDurationMS was computed from, folded
// into JobRuntime.DurationSumMS so app.updateStats continues the same exact
// sum instead of restarting from a value it would have to reverse-multiply.
DurationSumMS int64
} }
// SeedStats scans logsDir once and reconstructs per-job execution-time // SeedStats scans logsDir once and reconstructs per-job execution-time
@@ -45,8 +49,8 @@ func SeedStats(logsDir string, jobs []domain.Job, maxFiles int) map[int]SeededSt
return result return result
} }
byID := make(map[int][]string) byID := make(map[int][]logSummary)
byName := make(map[string][]string) byName := make(map[string][]logSummary)
for _, entry := range entries { for _, entry := range entries {
if entry.IsDir() { if entry.IsDir() {
continue continue
@@ -55,9 +59,10 @@ func SeedStats(logsDir string, jobs []domain.Job, maxFiles int) map[int]SeededSt
if !strings.HasSuffix(strings.ToLower(name), ".log") { if !strings.HasSuffix(strings.ToLower(name), ".log") {
continue continue
} }
path := filepath.Join(logsDir, name) summary := readLogSummary(filepath.Join(logsDir, name))
if jobID, ok := readLogJobID(path); ok { summary.name = name
byID[jobID] = append(byID[jobID], name) if summary.hasJobID {
byID[summary.jobID] = append(byID[summary.jobID], summary)
continue continue
} }
base := name[:len(name)-len(".log")] base := name[:len(name)-len(".log")]
@@ -65,7 +70,7 @@ func SeedStats(logsDir string, jobs []domain.Job, maxFiles int) map[int]SeededSt
if idx < 0 { if idx < 0 {
continue continue
} }
byName[base[idx+1:]] = append(byName[base[idx+1:]], name) byName[base[idx+1:]] = append(byName[base[idx+1:]], summary)
} }
for _, job := range jobs { for _, job := range jobs {
@@ -76,80 +81,71 @@ func SeedStats(logsDir string, jobs []domain.Job, maxFiles int) map[int]SeededSt
if len(files) == 0 { if len(files) == 0 {
continue continue
} }
// The timestamp prefix sorts chronologically, so a lexical sort puts the // The timestamp prefix sorts chronologically, so a lexical sort by file
// oldest first; keep the newest maxFiles to honor the retention bound. // name puts the oldest first; keep the newest maxFiles to honor the
sort.Strings(files) // retention bound.
sort.Slice(files, func(i, j int) bool { return files[i].name < files[j].name })
if maxFiles > 0 && len(files) > maxFiles { if maxFiles > 0 && len(files) > maxFiles {
files = files[len(files)-maxFiles:] files = files[len(files)-maxFiles:]
} }
result[job.ID] = aggregateLogStats(logsDir, files) result[job.ID] = aggregateLogStats(files)
} }
return result return result
} }
// aggregateLogStats folds the header of each log file (oldest first) into one // aggregateLogStats folds the already-read header of each log file (oldest
// SeededStats. Files lacking a duration line contribute to the run/fail counts // first) into one SeededStats. Files lacking a duration line contribute to the
// but not to the duration aggregates. // run/fail counts but not to the duration aggregates.
func aggregateLogStats(logsDir string, files []string) SeededStats { func aggregateLogStats(files []logSummary) SeededStats {
var stats SeededStats var stats SeededStats
var durationSum int64 var durationSum int64
var durationCount int var durationCount int
for _, file := range files { for _, file := range files {
state, durationMS, hasDuration := readLogHeader(filepath.Join(logsDir, file))
stats.RunCount++ stats.RunCount++
if state == "Failed" { if file.state == "Failed" {
stats.FailCount++ stats.FailCount++
} }
if hasDuration { if file.hasDuration {
// Files are oldest first, so the last assignment is the newest run. // Files are oldest first, so the last assignment is the newest run.
stats.LastDurationMS = durationMS stats.LastDurationMS = file.durationMS
if durationMS > stats.MaxDurationMS { if file.durationMS > stats.MaxDurationMS {
stats.MaxDurationMS = durationMS stats.MaxDurationMS = file.durationMS
} }
durationSum += durationMS durationSum += file.durationMS
durationCount++ durationCount++
} }
} }
if durationCount > 0 { if durationCount > 0 {
stats.TimedRunCount = durationCount stats.TimedRunCount = durationCount
stats.DurationSumMS = durationSum
stats.AvgDurationMS = durationSum / int64(durationCount) stats.AvgDurationMS = durationSum / int64(durationCount)
} }
return stats return stats
} }
// readLogJobID reads the job_id field from a log file header. // logSummary is everything SeedStats needs from one run log: the file name it
func readLogJobID(path string) (int, bool) { // sorts by, which job wrote it, how the run ended, and how long it took.
file, err := os.Open(path) type logSummary struct {
if err != nil { name string
return 0, false jobID int
} hasJobID bool
defer file.Close() state string
durationMS int64
scanner := bufio.NewScanner(file) hasDuration bool
for scanner.Scan() {
line := scanner.Text()
if line == "" {
break
}
if rest, ok := strings.CutPrefix(line, "job_id: "); ok {
id, err := strconv.Atoi(strings.TrimSpace(rest))
if err != nil {
return 0, false
}
return id, true
}
}
return 0, false
} }
// readLogHeader reads the "state" and "duration" fields from a log file's // readLogSummary reads the job_id, state, and duration fields from a log file's
// header (the lines before the first blank line). hasDuration reports whether a // header (the lines before the first blank line) in a single pass, so seeding
// well-formed duration line was present, distinguishing a legacy duration-less // opens each log once rather than once to find its job and again to read its
// log from one that genuinely recorded a zero-millisecond run. // result. The has* flags report whether a well-formed line was present,
func readLogHeader(path string) (state string, durationMS int64, hasDuration bool) { // distinguishing a legacy log written before the field existed from one that
// genuinely recorded a zero value. An unreadable file yields a zero summary,
// which falls back to matching by the job name in the file name.
func readLogSummary(path string) logSummary {
var summary logSummary
file, err := os.Open(path) file, err := os.Open(path)
if err != nil { if err != nil {
return "", 0, false return summary
} }
defer file.Close() defer file.Close()
@@ -159,14 +155,19 @@ func readLogHeader(path string) (state string, durationMS int64, hasDuration boo
if line == "" { if line == "" {
break // end of header break // end of header
} }
if rest, ok := strings.CutPrefix(line, "state: "); ok { if rest, ok := strings.CutPrefix(line, "job_id: "); ok {
state = strings.TrimSpace(rest) if id, err := strconv.Atoi(strings.TrimSpace(rest)); err == nil {
summary.jobID = id
summary.hasJobID = true
}
} else if rest, ok := strings.CutPrefix(line, "state: "); ok {
summary.state = strings.TrimSpace(rest)
} else if rest, ok := strings.CutPrefix(line, "duration: "); ok { } else if rest, ok := strings.CutPrefix(line, "duration: "); ok {
if value, err := strconv.ParseInt(strings.TrimSpace(rest), 10, 64); err == nil { if value, err := strconv.ParseInt(strings.TrimSpace(rest), 10, 64); err == nil {
durationMS = value summary.durationMS = value
hasDuration = true summary.hasDuration = true
} }
} }
} }
return state, durationMS, hasDuration return summary
} }
+3
View File
@@ -61,6 +61,9 @@ func TestSeedStatsBasic(t *testing.T) {
if s.AvgDurationMS != 400 { if s.AvgDurationMS != 400 {
t.Errorf("AvgDurationMS = %d, want 400", s.AvgDurationMS) t.Errorf("AvgDurationMS = %d, want 400", s.AvgDurationMS)
} }
if s.DurationSumMS != 1200 {
t.Errorf("DurationSumMS = %d, want 1200", s.DurationSumMS)
}
} }
// TestSeedStatsDurationLessLegacyLog verifies that a log without a duration // TestSeedStatsDurationLessLegacyLog verifies that a log without a duration
+62 -17
View File
@@ -19,6 +19,13 @@ type Store struct {
// PeekKeepRunningInTray reads keep_running_in_tray from gosentry.json for startup // PeekKeepRunningInTray reads keep_running_in_tray from gosentry.json for startup
// decisions that must run before app.Open(). On error it returns the built-in // decisions that must run before app.Open(). On error it returns the built-in
// default. // default.
//
// Despite the name, this can write: loadOrCreateConfig creates gosentry.json
// with defaults on first run, the same as OpenStore does moments later when
// app.Open() parses the now-existing file again. The double parse and the
// write-on-read are both harmless — the second read just sees the file the
// first one created — but worth knowing before adding a third startup path
// that also wants an early look at the config.
func PeekKeepRunningInTray() bool { func PeekKeepRunningInTray() bool {
paths, err := ResolvePaths() paths, err := ResolvePaths()
if err != nil { if err != nil {
@@ -65,19 +72,47 @@ func OpenStore() (*Store, []domain.Job, error) {
return store, jobs, nil return store, jobs, nil
} }
func (s *Store) SaveConfig() error { // PrepareSaveConfig re-resolves the derived paths from the current config and
// snapshots everything the write needs, returning the write itself as a closure.
// It exists so a caller that guards the Store with its own lock can do the file
// I/O — a marshal, an fsync, and a rename — after releasing that lock: the
// snapshot cannot change under the closure, so running it unlocked is safe.
// Prepared writes must be run in the order they were prepared, or an older
// snapshot can land on top of a newer one.
func (s *Store) PrepareSaveConfig() func() error {
s.applyConfigPaths() s.applyConfigPaths()
if err := os.MkdirAll(s.Paths.AppDir, 0o755); err != nil { dir := s.Paths.AppDir
path := s.Paths.ConfigPath
config := s.Config
return func() error {
if err := os.MkdirAll(dir, 0o755); err != nil {
return err return err
} }
return writeJSON(s.Paths.ConfigPath, s.Config) return writeJSON(path, config)
}
}
// PrepareSaveJobs is PrepareSaveConfig for the jobs file. The jobs slice is
// copied, so the caller may keep mutating its own slice as soon as this returns.
func (s *Store) PrepareSaveJobs(jobs []domain.Job) func() error {
dir := s.Paths.JobsDir
path := s.Paths.JobsPath
snapshot := make([]domain.Job, len(jobs))
copy(snapshot, jobs)
return func() error {
if err := os.MkdirAll(dir, 0o755); err != nil {
return err
}
return writeJSON(path, domain.JobsFile{Jobs: snapshot})
}
}
func (s *Store) SaveConfig() error {
return s.PrepareSaveConfig()()
} }
func (s *Store) SaveJobs(jobs []domain.Job) error { func (s *Store) SaveJobs(jobs []domain.Job) error {
if err := os.MkdirAll(s.Paths.JobsDir, 0o755); err != nil { return s.PrepareSaveJobs(jobs)()
return err
}
return writeJSON(s.Paths.JobsPath, domain.JobsFile{Jobs: jobs})
} }
func loadOrCreateConfig(paths Paths) (domain.Config, error) { func loadOrCreateConfig(paths Paths) (domain.Config, error) {
@@ -116,12 +151,12 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) {
if strings.TrimSpace(config.LogsDir) == "" { if strings.TrimSpace(config.LogsDir) == "" {
config.LogsDir = "logs" config.LogsDir = "logs"
} }
if config.MaxLogFiles <= 0 { // MaxLogFiles and MaxLogAgeDays are deliberately not normalized: 0 means
config.MaxLogFiles = 100 // "keep everything" (see runner.CleanupLogs), not a missing value, so
} // backfilling it here would make that choice impossible to persist. A config
if config.MaxLogAgeDays <= 0 { // written before either field existed already carries 0 from json.Unmarshal
config.MaxLogAgeDays = 30 // leaving the DefaultConfig() value in config untouched, so old files still
} // pick up 100 / 30 without an explicit backfill.
if config.ExecutionMode == "" { if config.ExecutionMode == "" {
config.ExecutionMode = domain.ExecutionModeParallel config.ExecutionMode = domain.ExecutionModeParallel
} }
@@ -179,13 +214,18 @@ func loadOrCreateJobs(path string) ([]domain.Job, error) {
func normalizeJobs(jobs []domain.Job) { func normalizeJobs(jobs []domain.Job) {
next := 1 next := 1
seen := make(map[int]bool, len(jobs))
for index := range jobs { for index := range jobs {
job := &jobs[index] job := &jobs[index]
if job.ID <= 0 { if job.ID <= 0 || seen[job.ID] {
// IDs are assigned only when absent. Existing IDs stay stable because // IDs are assigned only when absent or already claimed by an earlier job
// History and future log associations use them to identify jobs. // in this file — a hand-edited jobs.json can carry two entries with the
// same ID, which would otherwise share one runtime, one schedule-cache
// entry, and one SeedStats bucket. Existing, unique IDs stay stable
// because History and future log associations use them to identify jobs.
job.ID = next job.ID = next
} }
seen[job.ID] = true
if job.ID >= next { if job.ID >= next {
next = job.ID + 1 next = job.ID + 1
} }
@@ -213,7 +253,12 @@ func normalizeJobs(jobs []domain.Job) {
// apply the same rule to a path the user has typed but not yet saved. // apply the same rule to a path the user has typed but not yet saved.
func ResolveConfiguredPath(appDir string, path string) string { func ResolveConfiguredPath(appDir string, path string) string {
if filepath.IsAbs(path) { if filepath.IsAbs(path) {
return path // Cleaned so two spellings of the same file (forward vs. backslashes, a
// trailing separator) resolve to the same string. UpdateSettings compares
// this against Paths.JobsPath to decide whether the jobs file is changing,
// so an uncleaned path here could trigger a spurious adoption against the
// file the app is already using.
return filepath.Clean(path)
} }
// Relative paths are resolved against the executable directory, not the // Relative paths are resolved against the executable directory, not the
// process working directory. This matches ResolvePaths and keeps shortcuts, // process working directory. This matches ResolvePaths and keeps shortcuts,
+74
View File
@@ -4,6 +4,7 @@ import (
"encoding/json" "encoding/json"
"os" "os"
"path/filepath" "path/filepath"
"runtime"
"strings" "strings"
"testing" "testing"
@@ -145,6 +146,50 @@ func TestNormalizeJobsFillsDefaults(t *testing.T) {
} }
} }
// TestNormalizeJobsReassignsDuplicateIDs pins the fix for a hand-edited
// jobs.json carrying two entries with the same ID: without reassignment both
// would share one JobRuntime, one schedule-cache entry, and one SeedStats
// bucket, so editing or deleting either would silently affect both.
func TestNormalizeJobsReassignsDuplicateIDs(t *testing.T) {
jobs := []domain.Job{
{ID: 5, Name: "First"},
{ID: 5, Name: "Second"},
{ID: 5, Name: "Third"},
}
normalizeJobs(jobs)
seen := make(map[int]bool, len(jobs))
for _, job := range jobs {
if seen[job.ID] {
t.Fatalf("ID %d assigned to more than one job after normalization: %+v", job.ID, jobs)
}
seen[job.ID] = true
}
if jobs[0].ID != 5 {
t.Errorf("first occurrence should keep its ID: got %d, want 5", jobs[0].ID)
}
if jobs[1].ID == 5 || jobs[2].ID == 5 {
t.Errorf("later duplicates should be reassigned away from 5: got %d, %d", jobs[1].ID, jobs[2].ID)
}
}
// TestResolveConfiguredPathCleansAbsolutePaths pins the fix for two spellings
// of the same absolute path (forward vs. backslashes) resolving to different
// strings: UpdateSettings compares this against Paths.JobsPath as strings to
// decide whether the jobs file is changing, so an uncleaned path here could
// trigger a spurious adoption against the file already in use.
func TestResolveConfiguredPathCleansAbsolutePaths(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("backslash vs. forward-slash spellings of the same path are a Windows-only ambiguity")
}
got := ResolveConfiguredPath(`C:\app`, "C:/data/jobs.json")
want := ResolveConfiguredPath(`C:\app`, `C:\data\jobs.json`)
if got != want {
t.Errorf("forward-slash and backslash spellings resolved differently: %q vs %q", got, want)
}
}
func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) { func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
paths := Paths{ paths := Paths{
@@ -183,6 +228,35 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
} }
} }
// TestLoadOrCreateConfigPreservesZeroRetentionLimits verifies that 0 in
// max_log_files / max_log_age_days is read back as 0 ("keep everything"), not
// backfilled to the 100 / 30 defaults, since a config that already has the
// field set is not the "field is missing" case loadOrCreateConfig backfills.
func TestLoadOrCreateConfigPreservesZeroRetentionLimits(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
want := domain.DefaultConfig()
want.MaxLogFiles = 0
want.MaxLogAgeDays = 0
if err := writeJSON(paths.ConfigPath, want); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.MaxLogFiles != 0 {
t.Errorf("MaxLogFiles: got %d, want 0 (unlimited)", got.MaxLogFiles)
}
if got.MaxLogAgeDays != 0 {
t.Errorf("MaxLogAgeDays: got %d, want 0 (unlimited)", got.MaxLogAgeDays)
}
}
// TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing // TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing
// jobs.json is created with the sample jobs from defaultJobs, so a new user // jobs.json is created with the sample jobs from defaultJobs, so a new user
// sees scheduled and manual execution without inventing a command. // sees scheduled and manual execution without inventing a command.
-18
View File
@@ -5,8 +5,6 @@ import (
"strings" "strings"
"time" "time"
"gitea.mixdep.ru/mix/gosentry/src/domain"
"fyne.io/fyne/v2" "fyne.io/fyne/v2"
"fyne.io/fyne/v2/container" "fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/theme"
@@ -26,22 +24,6 @@ func newEvent(jobID int, jobName string, state string, detail string) event {
} }
} }
func collectActivity(jobs []job, runtimes map[int]*domain.JobRuntime) []event {
var events []event
for _, current := range jobs {
// At startup this is usually empty because jobs.json does not persist
// runtime logs. The function still centralizes the merge for future
// history loading from log metadata.
if rt := runtimes[current.ID]; rt != nil {
events = append(events, rt.Logs...)
}
}
sort.SliceStable(events, func(left int, right int) bool {
return events[left].Time < events[right].Time
})
return events
}
// textWidth measures how wide s renders at the theme's current body text size. // textWidth measures how wide s renders at the theme's current body text size.
func textWidth(s string) float32 { func textWidth(s string) float32 {
return fyne.MeasureText(s, theme.TextSize(), fyne.TextStyle{}).Width return fyne.MeasureText(s, theme.TextSize(), fyne.TextStyle{}).Width
-27
View File
@@ -6,8 +6,6 @@ import (
"testing" "testing"
"time" "time"
"gitea.mixdep.ru/mix/gosentry/src/domain"
"fyne.io/fyne/v2" "fyne.io/fyne/v2"
"fyne.io/fyne/v2/test" "fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/widget" "fyne.io/fyne/v2/widget"
@@ -57,31 +55,6 @@ func TestIndexOfID(t *testing.T) {
} }
} }
func TestCollectActivityMergesAndSorts(t *testing.T) {
jobs := []job{
{ID: 1, Name: "A"},
{ID: 2, Name: "B"},
}
runtimes := map[int]*domain.JobRuntime{
1: {Logs: []domain.RunRecord{{Time: "2026-01-02 10:00:00", JobID: 1}}},
2: {Logs: []domain.RunRecord{{Time: "2026-01-01 09:00:00", JobID: 2}}},
}
got := collectActivity(jobs, runtimes)
if len(got) != 2 {
t.Fatalf("len = %d, want 2", len(got))
}
if got[0].Time != "2026-01-01 09:00:00" || got[1].Time != "2026-01-02 10:00:00" {
t.Errorf("sort order = %v, want ascending by Time", got)
}
}
func TestCollectActivitySkipsMissingRuntimes(t *testing.T) {
jobs := []job{{ID: 1, Name: "A"}}
if got := collectActivity(jobs, nil); len(got) != 0 {
t.Errorf("nil runtimes: got %v, want empty", got)
}
}
func TestHistoryCellText(t *testing.T) { func TestHistoryCellText(t *testing.T) {
events := []event{{ events := []event{{
Time: "2026-06-01 12:00:00", Time: "2026-06-01 12:00:00",
+153 -308
View File
@@ -1,8 +1,6 @@
package ui package ui
import ( import (
"fmt"
"gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/domain" "gitea.mixdep.ru/mix/gosentry/src/domain"
@@ -22,340 +20,187 @@ const noFolder = "No folder"
// view; this panel is a quick at-a-glance summary anchored below the output. // view; this panel is a quick at-a-glance summary anchored below the output.
const maxJobActivityRows = 3 const maxJobActivityRows = 3
// jobsView owns the Jobs tab: the widgets, the view-only preferences they draw
// (list mode and the scheduler pause label), and the jobsViewState the widgets
// read. It replaces a single constructor whose dozen closures shared seven
// mutable locals — the state each handler touches is now named on the struct
// rather than captured, and the invariants that used to be maintained by hand in
// five places live on jobsViewState.
type jobsView struct {
w fyne.Window
svc *app.Service
state *jobsViewState
dp *detailsPanel
list *widget.List
folderSelect *widget.Select
viewButton *widget.Button
stopAllButton *widget.Button
schedulerState *widget.Label
// listView and paused mirror Service-owned config so the widgets can be
// relabelled without a round trip. Both are re-read from the Service on every
// refresh; neither is a source of truth.
listView domain.JobListView
paused bool
}
// newJobsView builds the Jobs tab: list sidebar, details panel, and toolbar. // newJobsView builds the Jobs tab: list sidebar, details panel, and toolbar.
// It returns the assembled panel and a refresh function the caller invokes // It returns the assembled panel and a refresh function the caller invokes
// whenever the service state may have changed (e.g., from the event subscriber // whenever the service state may have changed (e.g., from the event subscriber
// in mainwindow.go). The refresh function re-reads the service snapshot and // in mainwindow.go). The refresh function re-reads the service snapshot and
// redraws all widgets in the jobs view; it does NOT touch history or settings. // redraws all widgets in the jobs view; it does NOT touch history or settings.
func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
jobs := svc.Jobs() config := svc.Config()
runtimes := make(map[int]*domain.JobRuntime, len(jobs)) v := &jobsView{
syncFromService := func() { w: w,
jobs = svc.Jobs() svc: svc,
for id := range runtimes { state: newJobsViewState(svc),
delete(runtimes, id) listView: config.JobListView,
paused: config.Paused,
} }
for _, current := range jobs { v.dp = newDetailsPanel(job{}, &domain.JobRuntime{}, config.OverlapPolicy, config.DefaultTimeoutSeconds)
if rt := svc.Runtime(current.ID); rt != nil { v.updateDetails()
runtimes[current.ID] = rt
} // Build order follows what refresh() touches: the folder select fires its
} // OnChanged from SetSelected below, which refreshes, so every widget that
} // refresh() reaches has to exist by then.
syncFromService() v.list = v.newList()
runtimeFor := func(index int) *domain.JobRuntime { v.viewButton = v.newViewToggle()
if index < 0 || index >= len(jobs) { globalControls := v.newGlobalControls()
return &domain.JobRuntime{} v.folderSelect = v.newFolderSelect()
} v.folderSelect.SetSelected(v.state.folder)
if rt := runtimes[jobs[index].ID]; rt != nil { v.syncListSelection()
return rt
} return v.assemble(globalControls), v.refresh
return &domain.JobRuntime{}
} }
selected := 0 // refresh re-reads the Service and redraws the whole view. It is the single
if len(jobs) == 0 { // entry point for "something changed": the toolbar handlers call it after a
selected = -1 // successful operation, and mainwindow's event observer calls it for everything
} // else.
selectedFolder := allFolders func (v *jobsView) refresh() {
schedulerPaused := svc.Store().Config.Paused v.state.sync()
listView := svc.Store().Config.JobListView // The pause state is Service-owned and can change from outside this view, so
filteredJobs := filteredJobIndexes(jobs, selectedFolder) // it is re-read here rather than mirrored from the tap handler alone — that is
// what makes this view a consumer of SchedulerStateChanged.
dp := newDetailsPanel(job{}, &domain.JobRuntime{}, svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) v.applySchedulerState(v.svc.Config().Paused)
if selected >= 0 { // updateDetails already ends in a d.logs.Refresh() (both its update and clear
dp.update(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) // paths do), so refreshing the activity list again here would redraw it twice
} else { // per call.
dp.clear() v.updateDetails()
v.list.Refresh()
v.syncListSelection()
} }
updateDetails := func(index int) { // updateDetails repopulates the details pane from the current selection.
if index < 0 || index >= len(jobs) { func (v *jobsView) updateDetails() {
// A folder filter can temporarily leave no selectable rows. Clearing current, ok := v.state.selected()
// the details panel avoids showing stale information for a hidden job. if !ok {
dp.clear() // A folder filter can temporarily leave no selectable rows. Clearing the
// details panel avoids showing stale information for a hidden job.
v.dp.clear()
return return
} }
selected = index // Overlap policy and the default timeout are global settings that can change
dp.update(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy, svc.Store().Config.DefaultTimeoutSeconds) // from the Settings tab while this view is open, so they are re-read on every
// update rather than captured once at construction.
config := v.svc.Config()
v.dp.update(current, v.state.runtime(current.ID), config.OverlapPolicy, config.DefaultTimeoutSeconds)
} }
// list and folderSelect are declared early so closures below can reference // syncListSelection points the list's highlight at the selected job. It is what
// them before the widget.NewList / widget.NewSelect calls assign the values. // keeps the highlight and the details pane describing the same job when the row
var list *widget.List // a job sits in moves — a job created or deleted above it, a folder filter
var folderSelect *widget.Select // applied, or a different jobs file adopted. widget.List.Select returns early
// when the row is already highlighted, so calling this on every refresh does not
refreshView := func() { // fight the user's scrolling.
syncFromService() func (v *jobsView) syncListSelection() {
filteredJobs = filteredJobIndexes(jobs, selectedFolder) row := v.state.displayRow()
updateDetails(selected) if row < 0 {
dp.logs.Refresh() v.list.UnselectAll()
if list != nil { return
list.Refresh()
} }
v.list.Select(row)
} }
// applyRowMode expresses the current view mode as visibility on the row's // rebuildFolders re-derives the folder filter's options from the current jobs.
// four labels. widget.List caches the row template's MinSize, and // Creating, editing, and deleting a job can all add or remove a folder.
// list.Refresh() re-creates the template and recomputes it, so hiding lines func (v *jobsView) rebuildFolders() {
// is what actually shrinks the rows: layout.NewCustomPaddedVBoxLayout and the v.folderSelect.Options = folderOptions(v.state.jobs)
// border layout both skip hidden children when measuring. v.folderSelect.Refresh()
applyRowMode := func(inlineStatus, meta, status fyne.CanvasObject) {
if listView.IsCompact() {
inlineStatus.Show()
meta.Hide()
status.Hide()
return
}
inlineStatus.Hide()
meta.Show()
status.Show()
} }
list = widget.NewList( // assemble puts the sidebar (global controls, folder filter, toolbar, list) and
func() int { return len(filteredJobs) }, // the details pane into the master/detail split the tab shows.
func() fyne.CanvasObject { func (v *jobsView) assemble(globalControls fyne.CanvasObject) fyne.CanvasObject {
name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
// Truncating stops a long name from pushing the compact row's status
// off the right-hand edge. Labels default to TextWrapOff, which grows
// the widget to fit instead.
name.Truncation = fyne.TextTruncateClip
inlineStatus := widget.NewLabel("status")
meta := widget.NewLabel("schedule")
status := widget.NewLabel("status")
applyRowMode(inlineStatus, meta, status)
nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name)
return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), nameLine, meta, status)
},
func(id widget.ListItemID, item fyne.CanvasObject) {
row := item.(*fyne.Container)
// NewBorder keeps the center object first and appends the border slots
// after it, so nameLine is [name, inlineStatus].
nameLine := row.Objects[0].(*fyne.Container)
name := nameLine.Objects[0].(*widget.Label)
inlineStatus := nameLine.Objects[1].(*widget.Label)
meta := row.Objects[1].(*widget.Label)
status := row.Objects[2].(*widget.Label)
current := jobs[filteredJobs[id]]
name.SetText(current.Name)
// Keep each row compact: folder, schedule, and command are shown in one
// metadata line so the left pane stays useful even with many jobs.
meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current))
statusText := app.StatusText(current, runtimes[current.ID])
status.SetText(statusText)
inlineStatus.SetText(statusText)
// A full Refresh reuses rows built under the previous mode, so
// visibility cannot be left to the create callback alone.
applyRowMode(inlineStatus, meta, status)
},
)
list.OnSelected = func(id widget.ListItemID) {
if id < 0 || id >= len(filteredJobs) {
updateDetails(-1)
return
}
updateDetails(filteredJobs[id])
}
if len(filteredJobs) > 0 && selected >= 0 {
list.Select(app.DisplayIndex(filteredJobs, selected))
}
folderSelect = widget.NewSelect(folderOptions(jobs), func(value string) {
if value == "" {
return
}
selectedFolder = value
filteredJobs = filteredJobIndexes(jobs, selectedFolder)
if len(filteredJobs) == 0 {
// The "No folder" filter is intentionally allowed to be empty. It is a
// real filter choice, not an error state, so the selection is cleared.
// This path returns without reaching refreshView(), so it is the one
// place the list has to be redrawn by hand.
selected = -1
updateDetails(-1)
list.Refresh()
return
}
selected = filteredJobs[0]
list.Select(0)
refreshView()
})
folderSelect.SetSelected(selectedFolder)
// viewToggleIcon pairs with viewToggleText: both name the action the button
// performs, not the state it is in, matching stopAllButton's convention.
viewToggleIcon := func(current domain.JobListView) fyne.Resource {
if current.IsCompact() {
return theme.ViewFullScreenIcon()
}
return theme.ListIcon()
}
viewButton := widget.NewButtonWithIcon(viewToggleText(listView), viewToggleIcon(listView), nil)
viewButton.OnTapped = func() {
next := nextJobListView(listView)
listView = next
if err := svc.SetJobListView(next); err != nil {
// Roll the mode back and leave the button as it was, so the button
// never claims a preference that did not reach disk.
listView = nextJobListView(next)
dialog.ShowError(err, w)
return
}
viewButton.SetText(viewToggleText(listView))
viewButton.SetIcon(viewToggleIcon(listView))
// Refresh re-creates the row template, which is what recomputes the
// cached row height for the new mode. Selection is untouched.
list.Refresh()
}
addButton := widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() {
showJobDialog(w, "New job", job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true}, func(saved job) {
created, err := svc.CreateJob(saved)
if err != nil {
dialog.ShowError(err, w)
return
}
syncFromService()
folderSelect.Options = folderOptions(jobs)
folderSelect.Refresh()
targetFolder := filterValue(created.Folder)
if selectedFolder != allFolders && selectedFolder != targetFolder {
selectedFolder = targetFolder
folderSelect.SetSelected(targetFolder)
}
selected = indexOfID(jobs, created.ID)
filteredJobs = filteredJobIndexes(jobs, selectedFolder)
list.Select(app.DisplayIndex(filteredJobs, selected))
refreshView()
})
})
editButton := widget.NewButtonWithIcon("Edit", theme.DocumentCreateIcon(), func() {
if selected < 0 || selected >= len(jobs) {
return
}
showJobDialog(w, "Edit job", jobs[selected], func(saved job) {
saved.ID = jobs[selected].ID
if err := svc.UpdateJob(saved); err != nil {
dialog.ShowError(err, w)
return
}
syncFromService()
folderSelect.Options = folderOptions(jobs)
folderSelect.Refresh()
refreshView()
})
})
runButton := widget.NewButtonWithIcon("Run now", theme.MediaPlayIcon(), func() {
if selected < 0 || selected >= len(jobs) {
return
}
// A manual run is allowed even while the scheduler is paused: pause only
// stops automatic scheduled runs, not the user's explicit "Run now".
if err := svc.RunNow(jobs[selected].ID); err != nil {
dialog.ShowError(err, w)
return
}
refreshView()
})
stopAllText, stopAllIcon := "Disable auto", theme.MediaPauseIcon()
if schedulerPaused {
stopAllText, stopAllIcon = "Enable auto", theme.MediaPlayIcon()
}
schedulerStateText := "Scheduler running"
if schedulerPaused {
schedulerStateText = "Scheduler paused"
}
schedulerState := widget.NewLabel(schedulerStateText)
stopAllButton := widget.NewButtonWithIcon(stopAllText, stopAllIcon, nil)
stopAllButton.OnTapped = func() {
// SetGlobalPause flips the pause flag, updates every job's next-run text,
// and emits the activity record the observer logs. Revert if the save fails.
schedulerPaused = !schedulerPaused
if err := svc.SetGlobalPause(schedulerPaused); err != nil {
schedulerPaused = !schedulerPaused
dialog.ShowError(err, w)
return
}
if schedulerPaused {
schedulerState.SetText("Scheduler paused")
stopAllButton.SetText("Enable auto")
stopAllButton.SetIcon(theme.MediaPlayIcon())
} else {
schedulerState.SetText("Scheduler running")
stopAllButton.SetText("Disable auto")
stopAllButton.SetIcon(theme.MediaPauseIcon())
}
refreshView()
}
pauseButton := widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() {
if selected < 0 || selected >= len(jobs) {
return
}
current := jobs[selected]
if err := svc.SetEnabled(current.ID, !current.Enabled); err != nil {
dialog.ShowError(err, w)
return
}
refreshView()
})
deleteButton := widget.NewButtonWithIcon("Delete", theme.DeleteIcon(), func() {
if selected < 0 || selected >= len(jobs) {
return
}
deleted := jobs[selected]
// Deletion is confirmed because jobs can represent real system actions.
// There is no undo yet, so accidental removal should require one more click.
dialog.ShowConfirm("Delete job", fmt.Sprintf("Delete %q?", deleted.Name), func(confirm bool) {
if !confirm {
return
}
if err := svc.DeleteJob(deleted.ID); err != nil {
dialog.ShowError(err, w)
return
}
syncFromService()
folderSelect.Options = folderOptions(jobs)
folderSelect.Refresh()
filteredJobs = filteredJobIndexes(jobs, selectedFolder)
if len(filteredJobs) == 0 && selectedFolder != allFolders {
selectedFolder = allFolders
folderSelect.SetSelected(allFolders)
filteredJobs = filteredJobIndexes(jobs, selectedFolder)
}
if len(filteredJobs) == 0 {
selected = -1
} else {
selected = filteredJobs[0]
}
if selected >= 0 {
list.Select(app.DisplayIndex(filteredJobs, selected))
}
refreshView()
}, w)
})
toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer())
// The row sits directly under the tab bar with no AppTabs inset, while the
// default VBox gap below it is one theme padding — add the same on top so
// the button is not flush against the tabs.
globalControls := container.New(
layout.NewCustomPaddedLayout(theme.Padding(), 0, 0, 0),
container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer()),
)
// The whole filter is one row: caption on the left, view toggle on the right, // The whole filter is one row: caption on the left, view toggle on the right,
// select filling what is left. The border layout gives both edges their // select filling what is left. The border layout gives both edges their
// MinSize, so the header is a line shorter than a stacked caption would make it. // MinSize, so the header is a line shorter than a stacked caption would make it.
folderCaption := widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}) folderCaption := widget.NewLabelWithStyle("Folder", fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
filterRow := container.NewBorder(nil, nil, folderCaption, viewButton, folderSelect) filterRow := container.NewBorder(nil, nil, folderCaption, v.viewButton, v.folderSelect)
sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, toolbar) sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, v.newToolbar())
sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list) sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, v.list)
// A split rather than a Border left slot: the border pinned the sidebar at its // A split rather than a Border left slot: the border pinned the sidebar at its
// MinSize forever, so the user could never trade list width for detail width. // MinSize forever, so the user could never trade list width for detail width.
// The divider lets either pane grow, and neither can be dragged below its own // The divider lets either pane grow, and neither can be dragged below its own
// content minimum. // content minimum.
panel := container.NewHSplit(sidebar, container.NewPadded(dp.container())) panel := container.NewHSplit(sidebar, container.NewPadded(v.dp.container()))
panel.SetOffset(initialSplitOffset(sidebar.MinSize().Width)) panel.SetOffset(initialSplitOffset(sidebar.MinSize().Width))
return panel, refreshView return panel
}
// newFolderSelect builds the folder filter. Selecting a folder narrows the list
// and, when the selected job is no longer visible, moves the selection to the
// first row that is (see jobsViewState.applyFilter).
func (v *jobsView) newFolderSelect() *widget.Select {
return widget.NewSelect(folderOptions(v.state.jobs), func(value string) {
if value == "" {
return
}
v.state.applyFilter(value)
v.refresh()
})
}
// newGlobalControls builds the pause control row that sits above the filter.
func (v *jobsView) newGlobalControls() fyne.CanvasObject {
v.schedulerState = widget.NewLabel("")
v.stopAllButton = widget.NewButtonWithIcon("", nil, nil)
v.applySchedulerState(v.paused)
v.stopAllButton.OnTapped = func() {
// SetGlobalPause flips the pause flag, updates every job's next-run text,
// and emits the activity record the observer logs. refresh re-derives the
// pause state from the Service, so a failed save leaves the control showing
// what actually happened.
if err := v.svc.SetGlobalPause(!v.paused); err != nil {
dialog.ShowError(err, v.w)
return
}
v.refresh()
}
// The row sits directly under the tab bar with no AppTabs inset, while the
// default VBox gap below it is one theme padding — add the same on top so
// the button is not flush against the tabs.
return container.New(
layout.NewCustomPaddedLayout(theme.Padding(), 0, 0, 0),
container.NewHBox(v.stopAllButton, v.schedulerState, layout.NewSpacer()),
)
}
// applySchedulerState is the one place that draws the pause control and its
// status text from a pause value, so refresh can drive it from whatever the
// Service reports instead of only the tap handler mirroring its own toggle.
func (v *jobsView) applySchedulerState(paused bool) {
v.paused = paused
if paused {
v.schedulerState.SetText("Scheduler paused")
v.stopAllButton.SetText("Enable auto")
v.stopAllButton.SetIcon(theme.MediaPlayIcon())
return
}
v.schedulerState.SetText("Scheduler running")
v.stopAllButton.SetText("Disable auto")
v.stopAllButton.SetIcon(theme.MediaPauseIcon())
} }
+1 -1
View File
@@ -8,7 +8,7 @@ import (
// lastJobLogs returns a fresh slice of the most recent activity entries for the // lastJobLogs returns a fresh slice of the most recent activity entries for the
// "Selected job activity" panel. Logs are stored newest-first (see // "Selected job activity" panel. Logs are stored newest-first (see
// app.Service.recordRun), so the leading entries are the latest; the result is // app.prependLog), so the leading entries are the latest; the result is
// capped at maxJobActivityRows. // capped at maxJobActivityRows.
func lastJobLogs(logs []event) []event { func lastJobLogs(logs []event) []event {
n := len(logs) n := len(logs)
+114
View File
@@ -0,0 +1,114 @@
package ui
import (
"gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/domain"
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/dialog"
"fyne.io/fyne/v2/layout"
"fyne.io/fyne/v2/theme"
"fyne.io/fyne/v2/widget"
)
// newList builds the sidebar's job list. Rows are drawn from jobsViewState's
// filtered view, so the row index the widget reports is a position in the
// filter, never an index into the job snapshot.
func (v *jobsView) newList() *widget.List {
list := widget.NewList(
func() int { return len(v.state.filtered) },
func() fyne.CanvasObject {
name := widget.NewLabelWithStyle("Job name", fyne.TextAlignLeading, fyne.TextStyle{Bold: true})
// Truncating stops a long name from pushing the compact row's status
// off the right-hand edge. Labels default to TextWrapOff, which grows
// the widget to fit instead.
name.Truncation = fyne.TextTruncateClip
inlineStatus := widget.NewLabel("status")
meta := widget.NewLabel("schedule")
status := widget.NewLabel("status")
v.applyRowMode(inlineStatus, meta, status)
nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name)
return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), nameLine, meta, status)
},
func(id widget.ListItemID, item fyne.CanvasObject) {
current, ok := v.state.jobAt(int(id))
if !ok {
return
}
row := item.(*fyne.Container)
// NewBorder keeps the center object first and appends the border slots
// after it, so nameLine is [name, inlineStatus].
nameLine := row.Objects[0].(*fyne.Container)
name := nameLine.Objects[0].(*widget.Label)
inlineStatus := nameLine.Objects[1].(*widget.Label)
meta := row.Objects[1].(*widget.Label)
status := row.Objects[2].(*widget.Label)
name.SetText(current.Name)
// Keep each row compact: folder, schedule, and command are shown in one
// metadata line so the left pane stays useful even with many jobs.
meta.SetText(app.DisplayFolder(current.Folder) + " " + current.Schedule + " " + app.DisplayInvocation(current))
statusText := app.StatusText(current, v.state.runtime(current.ID))
status.SetText(statusText)
inlineStatus.SetText(statusText)
// A full Refresh reuses rows built under the previous mode, so
// visibility cannot be left to the create callback alone.
v.applyRowMode(inlineStatus, meta, status)
},
)
list.OnSelected = func(id widget.ListItemID) {
v.state.selectRow(int(id))
v.updateDetails()
}
return list
}
// applyRowMode expresses the current view mode as visibility on the row's
// four labels. widget.List caches the row template's MinSize, and
// list.Refresh() re-creates the template and recomputes it, so hiding lines
// is what actually shrinks the rows: layout.NewCustomPaddedVBoxLayout and the
// border layout both skip hidden children when measuring.
func (v *jobsView) applyRowMode(inlineStatus, meta, status fyne.CanvasObject) {
if v.listView.IsCompact() {
inlineStatus.Show()
meta.Hide()
status.Hide()
return
}
inlineStatus.Hide()
meta.Show()
status.Show()
}
// newViewToggle builds the compact/detailed switch that sits at the right edge
// of the filter row.
func (v *jobsView) newViewToggle() *widget.Button {
button := widget.NewButtonWithIcon(viewToggleText(v.listView), viewToggleIcon(v.listView), nil)
button.OnTapped = func() {
next := nextJobListView(v.listView)
v.listView = next
if err := v.svc.SetJobListView(next); err != nil {
// Roll the mode back and leave the button as it was, so the button
// never claims a preference that did not reach disk.
v.listView = nextJobListView(next)
dialog.ShowError(err, v.w)
return
}
button.SetText(viewToggleText(v.listView))
button.SetIcon(viewToggleIcon(v.listView))
// Refresh re-creates the row template, which is what recomputes the
// cached row height for the new mode. Selection is untouched.
v.list.Refresh()
}
return button
}
// viewToggleIcon pairs with viewToggleText: both name the action the button
// performs, not the state it is in, matching stopAllButton's convention.
func viewToggleIcon(current domain.JobListView) fyne.Resource {
if current.IsCompact() {
return theme.ViewFullScreenIcon()
}
return theme.ListIcon()
}
+156
View File
@@ -0,0 +1,156 @@
package ui
import (
"gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/domain"
)
// jobsViewState is the model behind the Jobs tab: the snapshot of the Service's
// jobs and runtimes, the folder filter, and the selection. The widgets in
// jobsView read it and never keep a second copy of any of it.
//
// The selection is a job ID, not an index into the snapshot. Every path that
// changes the job list replaces that snapshot underneath the view — create,
// delete, and edit do it from this view's own handlers, but adopting a different
// jobs file does it from the Service, and the view only learns about it through
// the refresh that JobsLoaded triggers. An index that outlives its snapshot then
// points at whichever job happens to sit there now, so the details pane
// describes one job while the list highlights another. Indexes are derived from
// the ID at render time instead (selectedIndex, displayRow).
type jobsViewState struct {
svc *app.Service
jobs []job
runtimes map[int]*domain.JobRuntime
folder string
// selectedID is 0 when nothing is selected; job IDs start at 1.
selectedID int
// filtered holds the indexes into jobs that the folder filter shows, in list
// row order: filtered[row] is the index of the job drawn in that row.
filtered []int
}
func newJobsViewState(svc *app.Service) *jobsViewState {
s := &jobsViewState{
svc: svc,
runtimes: map[int]*domain.JobRuntime{},
folder: allFolders,
}
s.sync()
return s
}
// sync re-reads the Service snapshot, re-applies the folder filter, and
// re-resolves the selection against the new list. It is the only place the view
// reads job state from the Service.
func (s *jobsViewState) sync() {
s.jobs = s.svc.Jobs()
clear(s.runtimes)
for _, current := range s.jobs {
if rt := s.svc.Runtime(current.ID); rt != nil {
s.runtimes[current.ID] = rt
}
}
s.filtered = filteredJobIndexes(s.jobs, s.folder)
s.resolveSelection()
}
// applyFilter switches the folder filter, keeping the current selection when the
// new filter still shows it. A filter that matches nothing — "No folder" with no
// such job — is a real filter choice, not an error state, so it simply leaves
// nothing selected.
func (s *jobsViewState) applyFilter(folder string) {
s.folder = folder
s.filtered = filteredJobIndexes(s.jobs, s.folder)
if !s.visible(s.selectedID) {
s.selectedID = 0
}
s.resolveSelection()
}
// resolveSelection drops a selection whose job is gone and falls back to the
// first visible row, so the details pane never describes a job the current
// snapshot no longer holds.
func (s *jobsViewState) resolveSelection() {
if s.selectedID != 0 && indexOfID(s.jobs, s.selectedID) < 0 {
s.selectedID = 0
}
if s.selectedID == 0 && len(s.filtered) > 0 {
s.selectedID = s.jobs[s.filtered[0]].ID
}
}
// selectByID records the selection directly, for handlers that know the job they
// want selected (a newly created job, for instance) rather than its row.
func (s *jobsViewState) selectByID(id int) {
s.selectedID = id
}
// selectRow records the selection from a list row, which is what widget.List
// reports through OnSelected.
func (s *jobsViewState) selectRow(row int) {
current, ok := s.jobAt(row)
if !ok {
s.selectedID = 0
return
}
s.selectedID = current.ID
}
// selected returns the selected job, or false when nothing is selected.
func (s *jobsViewState) selected() (job, bool) {
index := s.selectedIndex()
if index < 0 {
return job{}, false
}
return s.jobs[index], true
}
// selectedIndex resolves the selected ID to an index into the current snapshot,
// or -1 when nothing is selected.
func (s *jobsViewState) selectedIndex() int {
if s.selectedID == 0 {
return -1
}
return indexOfID(s.jobs, s.selectedID)
}
// displayRow maps the selection onto a list row, or -1 when nothing is selected
// or the filter hides the selected job — so a caller unselects rather than
// highlighting an unrelated row.
func (s *jobsViewState) displayRow() int {
index := s.selectedIndex()
if index < 0 || !s.visible(s.selectedID) {
return -1
}
return app.DisplayIndex(s.filtered, index)
}
// jobAt returns the job drawn in the given list row.
func (s *jobsViewState) jobAt(row int) (job, bool) {
if row < 0 || row >= len(s.filtered) {
return job{}, false
}
return s.jobs[s.filtered[row]], true
}
// runtime returns a job's runtime, or an empty one when the Service has none
// yet, so callers can read it without a nil check.
func (s *jobsViewState) runtime(id int) *domain.JobRuntime {
if rt := s.runtimes[id]; rt != nil {
return rt
}
return &domain.JobRuntime{}
}
// visible reports whether the folder filter shows the given job.
func (s *jobsViewState) visible(id int) bool {
if id == 0 {
return false
}
for _, index := range s.filtered {
if s.jobs[index].ID == id {
return true
}
}
return false
}
+198
View File
@@ -0,0 +1,198 @@
package ui
import (
"testing"
"gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/domain"
)
// newStateForTest builds a jobsViewState over a Service holding the given jobs.
// No Fyne app is needed: the state is the view's model and touches no widgets.
func newStateForTest(t *testing.T, jobs []domain.Job) (*jobsViewState, *app.Service) {
t.Helper()
svc := app.NewService(newTestStore(t), jobs)
t.Cleanup(svc.Stop)
return newJobsViewState(svc), svc
}
func threeJobs() []domain.Job {
return []domain.Job{
{ID: 1, Name: "First", Folder: "Maintenance", Schedule: "@every 1m", Command: "echo one", Enabled: true},
{ID: 2, Name: "Second", Schedule: "@every 2m", Command: "echo two", Enabled: true},
{ID: 3, Name: "Third", Folder: "Reports", Schedule: "@every 3m", Command: "echo three", Enabled: true},
}
}
func selectedName(t *testing.T, s *jobsViewState) string {
t.Helper()
current, ok := s.selected()
if !ok {
return ""
}
return current.Name
}
// TestJobsViewStateSelectsTheFirstJob pins the opening state: the first row is
// selected so the details pane is never blank when there is something to show.
func TestJobsViewStateSelectsTheFirstJob(t *testing.T) {
s, _ := newStateForTest(t, threeJobs())
if got := selectedName(t, s); got != "First" {
t.Errorf("selected job = %q, want %q", got, "First")
}
if got := s.displayRow(); got != 0 {
t.Errorf("displayRow = %d, want 0", got)
}
}
func TestJobsViewStateEmptyListSelectsNothing(t *testing.T) {
s, _ := newStateForTest(t, nil)
if _, ok := s.selected(); ok {
t.Error("an empty job list should leave nothing selected")
}
if got := s.displayRow(); got != -1 {
t.Errorf("displayRow with nothing selected = %d, want -1", got)
}
}
// TestJobsViewStateSelectionFollowsTheJobNotTheRow is the regression guard for
// the selection defect: the selection is a job ID, so a job removed above the
// selected one must not slide the selection onto its neighbour. The deletion
// goes through the Service rather than the Delete button, which is how the view
// learns about a job list that changed underneath it (a different jobs file
// adopted, or any other broad JobChanged).
func TestJobsViewStateSelectionFollowsTheJobNotTheRow(t *testing.T) {
s, svc := newStateForTest(t, threeJobs())
s.selectRow(2)
if got := selectedName(t, s); got != "Third" {
t.Fatalf("selected job after selecting row 2 = %q, want %q", got, "Third")
}
if err := svc.DeleteJob(1); err != nil {
t.Fatalf("DeleteJob: %v", err)
}
s.sync()
if got := selectedName(t, s); got != "Third" {
t.Errorf("selected job after the first job was removed = %q, want it still on %q", got, "Third")
}
if got := s.displayRow(); got != 1 {
t.Errorf("displayRow = %d, want the row %q moved to (1)", got, "Third")
}
}
// TestJobsViewStateDropsSelectionWhenItsJobIsGone covers the other half: a
// selected job that no longer exists falls back to the first visible row instead
// of describing whichever job inherited its position.
func TestJobsViewStateDropsSelectionWhenItsJobIsGone(t *testing.T) {
s, svc := newStateForTest(t, threeJobs())
s.selectRow(1)
if err := svc.DeleteJob(2); err != nil {
t.Fatalf("DeleteJob: %v", err)
}
s.sync()
if got := selectedName(t, s); got != "First" {
t.Errorf("selected job after deleting the selected one = %q, want the fallback %q", got, "First")
}
}
func TestJobsViewStateApplyFilter(t *testing.T) {
s, _ := newStateForTest(t, threeJobs())
// The selected job is in the folder being filtered to, so it stays selected.
s.selectRow(2)
s.applyFilter("Reports")
if got := selectedName(t, s); got != "Third" {
t.Errorf("selection after filtering to its own folder = %q, want %q", got, "Third")
}
if got := s.displayRow(); got != 0 {
t.Errorf("displayRow inside the filter = %d, want 0", got)
}
// Filtering to a folder that hides it moves the selection to the first row
// that folder does show.
s.applyFilter("Maintenance")
if got := selectedName(t, s); got != "First" {
t.Errorf("selection after filtering it away = %q, want %q", got, "First")
}
// "No folder" matches the one job without one.
s.applyFilter(noFolder)
if got := selectedName(t, s); got != "Second" {
t.Errorf("selection under the %q filter = %q, want %q", noFolder, got, "Second")
}
s.applyFilter(allFolders)
if got := len(s.filtered); got != 3 {
t.Errorf("rows under %q = %d, want 3", allFolders, got)
}
}
// TestJobsViewStateEmptyFilterSelectsNothing pins that a filter matching no job
// is a filter choice, not an error state: nothing is selected, and nothing is
// highlighted either.
func TestJobsViewStateEmptyFilterSelectsNothing(t *testing.T) {
s, _ := newStateForTest(t, []domain.Job{
{ID: 1, Name: "First", Folder: "Maintenance", Schedule: "@every 1m", Command: "echo one", Enabled: true},
})
s.applyFilter(noFolder)
if _, ok := s.selected(); ok {
t.Error("a filter that matches nothing should leave nothing selected")
}
if got := s.displayRow(); got != -1 {
t.Errorf("displayRow under an empty filter = %d, want -1", got)
}
// The selection comes back when the filter does.
s.applyFilter(allFolders)
if got := selectedName(t, s); got != "First" {
t.Errorf("selection after clearing the filter = %q, want %q", got, "First")
}
}
// TestJobsViewStateHiddenSelectionIsNotHighlighted covers the case the list
// widget cannot express: the selected job still exists but the filter hides it,
// so there is no row to highlight and displayRow must say so rather than fall
// back to row 0.
func TestJobsViewStateHiddenSelectionIsNotHighlighted(t *testing.T) {
s, _ := newStateForTest(t, threeJobs())
s.applyFilter("Maintenance")
// Selecting by ID is how the create handler points the view at a job it just
// made; here it reaches the state a hidden-but-selected job would be in.
s.selectByID(3)
if s.visible(3) {
t.Fatal("job 3 should be hidden by the Maintenance filter")
}
if got := s.displayRow(); got != -1 {
t.Errorf("displayRow for a hidden selection = %d, want -1", got)
}
if got := selectedName(t, s); got != "Third" {
t.Errorf("selected job = %q, want it still %q", got, "Third")
}
}
func TestJobsViewStateRuntimeIsNeverNil(t *testing.T) {
s, _ := newStateForTest(t, threeJobs())
if rt := s.runtime(99); rt == nil {
t.Error("runtime for an unknown job returned nil, want an empty runtime")
}
}
func TestJobsViewStateJobAtRejectsRowsOutsideTheFilter(t *testing.T) {
s, _ := newStateForTest(t, threeJobs())
s.applyFilter("Reports")
if current, ok := s.jobAt(0); !ok || current.Name != "Third" {
t.Errorf("jobAt(0) = (%q, %v), want (%q, true)", current.Name, ok, "Third")
}
if _, ok := s.jobAt(1); ok {
t.Error("jobAt past the last filtered row should report no job")
}
if _, ok := s.jobAt(-1); ok {
t.Error("jobAt(-1) should report no job")
}
}
+76 -1
View File
@@ -1,6 +1,9 @@
package ui package ui
import ( import (
"encoding/json"
"os"
"path/filepath"
"testing" "testing"
"gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/app"
@@ -378,7 +381,7 @@ func TestJobsSplitOpensAtTheSidebarWidth(t *testing.T) {
// TestToolbarButtonRedrawsRowAndDetails is the regression guard for F12: the // TestToolbarButtonRedrawsRowAndDetails is the regression guard for F12: the
// toolbar handlers no longer re-read the service or refresh the list // toolbar handlers no longer re-read the service or refresh the list
// themselves, so refreshView alone has to re-snapshot the jobs and repopulate // themselves, so jobsView.refresh alone has to re-snapshot the jobs and repopulate
// the details pane. If it ever stops doing either, the row renders a stale // the details pane. If it ever stops doing either, the row renders a stale
// status and the details lose the selection — neither is a compile error. // status and the details lose the selection — neither is a compile error.
func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) { func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) {
@@ -441,6 +444,78 @@ func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) {
} }
} }
// TestJobsViewSelectionSurvivesAJobsFileSwitch is the view-level regression
// guard for the selection defect. Adopting a different jobs file replaces the
// whole list from the Service; the view only hears about it through the refresh
// that JobsLoaded triggers, which is exactly what this test calls. With the
// selection held as a row index, that refresh redrew the details pane from the
// old index — describing whichever job now sat there, or clearing the pane when
// the new list was shorter — while the list's highlight stayed where it was.
func TestJobsViewSelectionSurvivesAJobsFileSwitch(t *testing.T) {
testApp := test.NewApp()
defer testApp.Quit()
w := testApp.NewWindow("test")
defer w.Close()
store := newTestStore(t)
svc := app.NewService(store, []domain.Job{
{ID: 1, Name: "First", Schedule: "@every 1m", Command: "echo one", Enabled: true},
{ID: 2, Name: "Second", Schedule: "@every 2m", Command: "echo two", Enabled: true},
{ID: 3, Name: "Third", Schedule: "@every 3m", Command: "echo three", Enabled: true},
})
defer svc.Stop()
content, refresh := newJobsView(w, svc)
w.SetContent(content)
list := jobsList(t, content)
list.Select(2)
if got := jobsDetailsTitle(t, content); got != "Third" {
t.Fatalf("details title after selecting row 2 = %q, want %q", got, "Third")
}
// A second jobs file with different jobs and different IDs, so nothing about
// the old selection can resolve into the new list.
other := []domain.Job{
{ID: 10, Name: "Alpha", Schedule: "@every 1m", Command: "echo alpha", Enabled: true},
{ID: 11, Name: "Beta", Schedule: "@every 2m", Command: "echo beta", Enabled: true},
}
payload, err := json.Marshal(domain.JobsFile{Jobs: other})
if err != nil {
t.Fatalf("marshal jobs: %v", err)
}
if err := os.WriteFile(filepath.Join(store.Paths.AppDir, "other.json"), payload, 0o644); err != nil {
t.Fatalf("write jobs file: %v", err)
}
config := svc.Config()
config.JobsFile = "other.json"
if err := svc.UpdateSettings(config); err != nil {
t.Fatalf("UpdateSettings: %v", err)
}
refresh()
if got := jobsDetailsTitle(t, content); got != "Alpha" {
t.Errorf("details title after the switch = %q, want the first job of the new file %q", got, "Alpha")
}
if got := list.Length(); got != len(other) {
t.Fatalf("list length after the switch = %d, want %d", got, len(other))
}
// widget.List.Select returns without calling OnSelected when the row is
// already highlighted, so a silent Select(0) is what proves the highlight and
// the details pane are describing the same job.
reselected := false
inner := list.OnSelected
list.OnSelected = func(id widget.ListItemID) {
reselected = true
inner(id)
}
defer func() { list.OnSelected = inner }()
list.Select(0)
if reselected {
t.Error("row 0 was not the highlighted row after the switch, so the highlight and the details pane disagree")
}
}
// TestDetailCaptionWidthCoversEveryCaption is the guard that makes the single // TestDetailCaptionWidthCoversEveryCaption is the guard that makes the single
// metadataRows list self-enforcing (F10): every caption it returns must // metadataRows list self-enforcing (F10): every caption it returns must
// measure no wider than captionColumnWidth's result for that same list, or a // measure no wider than captionColumnWidth's result for that same list, or a
+136
View File
@@ -0,0 +1,136 @@
package ui
import (
"fmt"
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/dialog"
"fyne.io/fyne/v2/layout"
"fyne.io/fyne/v2/theme"
"fyne.io/fyne/v2/widget"
)
// newToolbar builds the per-job button row under the folder filter. Every
// handler works from the selected job — never from a row index — and ends in
// refresh, which is what re-reads the Service and redraws the row, the details
// pane, and the list highlight.
func (v *jobsView) newToolbar() fyne.CanvasObject {
return container.NewHBox(
v.newAddButton(),
v.newEditButton(),
v.newRunButton(),
v.newPauseButton(),
v.newDeleteButton(),
layout.NewSpacer(),
)
}
func (v *jobsView) newAddButton() *widget.Button {
return widget.NewButtonWithIcon("New job", theme.ContentAddIcon(), func() {
blank := job{Schedule: "@every 1m", Command: "echo GoSentry job ran", Enabled: true}
showJobDialog(v.w, "New job", blank, func(saved job) {
created, err := v.svc.CreateJob(saved)
if err != nil {
dialog.ShowError(err, v.w)
return
}
v.state.sync()
// The new job may have introduced a folder, so the options are rebuilt
// before the filter is pointed at it.
v.rebuildFolders()
v.state.selectByID(created.ID)
if target := filterValue(created.Folder); v.state.folder != allFolders && v.state.folder != target {
// The current filter would hide the job the user just created. Switch
// to its folder; SetSelected fires OnChanged, which applies the filter
// and refreshes.
v.folderSelect.SetSelected(target)
}
v.refresh()
})
})
}
func (v *jobsView) newEditButton() *widget.Button {
return widget.NewButtonWithIcon("Edit", theme.DocumentCreateIcon(), func() {
current, ok := v.state.selected()
if !ok {
return
}
showJobDialog(v.w, "Edit job", current, func(saved job) {
// The ID comes from the job the dialog was opened on, so a list that
// changed underneath the open dialog cannot redirect the save.
saved.ID = current.ID
if err := v.svc.UpdateJob(saved); err != nil {
dialog.ShowError(err, v.w)
return
}
v.state.sync()
// An edit can rename the job's folder, add a new one, or empty the last
// job out of an existing one.
v.rebuildFolders()
v.refresh()
})
})
}
func (v *jobsView) newRunButton() *widget.Button {
return widget.NewButtonWithIcon("Run now", theme.MediaPlayIcon(), func() {
current, ok := v.state.selected()
if !ok {
return
}
// A manual run is allowed even while the scheduler is paused: pause only
// stops automatic scheduled runs, not the user's explicit "Run now".
if err := v.svc.RunNow(current.ID); err != nil {
dialog.ShowError(err, v.w)
return
}
v.refresh()
})
}
func (v *jobsView) newPauseButton() *widget.Button {
return widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() {
current, ok := v.state.selected()
if !ok {
return
}
if err := v.svc.SetEnabled(current.ID, !current.Enabled); err != nil {
dialog.ShowError(err, v.w)
return
}
v.refresh()
})
}
func (v *jobsView) newDeleteButton() *widget.Button {
return widget.NewButtonWithIcon("Delete", theme.DeleteIcon(), func() {
deleted, ok := v.state.selected()
if !ok {
return
}
// Deletion is confirmed because jobs can represent real system actions.
// There is no undo yet, so accidental removal should require one more click.
dialog.ShowConfirm("Delete job", fmt.Sprintf("Delete %q?", deleted.Name), func(confirm bool) {
if !confirm {
return
}
if err := v.svc.DeleteJob(deleted.ID); err != nil {
dialog.ShowError(err, v.w)
return
}
// sync drops the deleted job's selection and falls back to the first row
// the filter still shows.
v.state.sync()
v.rebuildFolders()
if len(v.state.filtered) == 0 && v.state.folder != allFolders {
// The deleted job was the last one in its folder, and that folder is
// no longer an option. Fall back to "All" rather than leaving the user
// on an empty filter they did not choose.
v.folderSelect.SetSelected(allFolders)
}
v.refresh()
}, v.w)
})
}
+26 -33
View File
@@ -21,20 +21,11 @@ const runRecordTimeLayout = "2006-01-02 15:04:05"
type job = domain.Job type job = domain.Job
type event = domain.RunRecord type event = domain.RunRecord
func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time.Duration, bool)) { func newMainView(w fyne.Window, svc *app.Service, tray *trayState) (fyne.CanvasObject, func(time.Duration, bool)) {
svc.InstallDesktopIcon(appID, assets.IconBytes()) // History is session-only: jobs.json never persists JobRuntime.Logs (see
// domain.JobRuntime), so there is nothing to seed the History tab with at
// Build the initial event history from the current runtime state. Jobs and // startup. It starts empty and fills as events arrive.
// runtimes are read here only for this one-time initialization; the jobs view events := newHistoryLog(nil)
// owns all subsequent state via its own syncFromService closure.
initialJobs := svc.Jobs()
initialRuntimes := make(map[int]*domain.JobRuntime, len(initialJobs))
for _, j := range initialJobs {
if rt := svc.Runtime(j.ID); rt != nil {
initialRuntimes[j.ID] = rt
}
}
events := newHistoryLog(collectActivity(initialJobs, initialRuntimes))
jobsPanel, refreshJobsView := newJobsView(w, svc) jobsPanel, refreshJobsView := newJobsView(w, svc)
@@ -65,54 +56,56 @@ func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time.
// the main thread in both cases, so the engine never mutates Fyne state off // the main thread in both cases, so the engine never mutates Fyne state off
// the UI thread. This is the sole place events touch widgets. (Resolves #4.) // the UI thread. This is the sole place events touch widgets. (Resolves #4.)
svc.Subscribe(app.ObserverFunc(func(ev app.Event) { svc.Subscribe(app.ObserverFunc(func(ev app.Event) {
recorded, isRecorded := ev.(app.RunRecorded)
errOccurred, isError := ev.(app.ErrorOccurred)
jobsLoaded, isJobsLoaded := ev.(app.JobsLoaded)
fyne.Do(func() { fyne.Do(func() {
if isRecorded { // A type switch does not get compiler-enforced exhaustiveness (see
events.add(recorded.Record) // app.Event's doc comment) — JobChanged and SchedulerStateChanged
r := recorded.Record // intentionally fall through to the unconditional refresh() below
if r.State == "Failed" && // without their own case, since a broad state re-read is all they need.
(r.Trigger == "Manual" || r.Trigger == "Schedule") && switch e := ev.(type) {
case app.RunRecorded:
events.add(e.Record)
if e.Record.State == "Failed" &&
(e.Record.Trigger == "Manual" || e.Record.Trigger == "Schedule") &&
svc.ShouldNotifyOnFailure() { svc.ShouldNotifyOnFailure() {
timing := notificationTiming{ timing := notificationTiming{
JobName: r.JobName, JobName: e.Record.JobName,
EmittedAt: time.Now(), EmittedAt: time.Now(),
} }
if finished, err := time.ParseInLocation(runRecordTimeLayout, r.Time, time.Local); err == nil { if finished, err := time.ParseInLocation(runRecordTimeLayout, e.Record.Time, time.Local); err == nil {
timing.RunFinished = finished timing.RunFinished = finished
} }
fyne.Do(func() { fyne.Do(func() {
timing.UIQueuedAt = time.Now() timing.UIQueuedAt = time.Now()
fyne.CurrentApp().SendNotification(&fyne.Notification{ fyne.CurrentApp().SendNotification(&fyne.Notification{
Title: "GoSentry: Job Failed", Title: "GoSentry: Job Failed",
Content: r.JobName + ": " + r.Detail, Content: e.Record.JobName + ": " + e.Record.Detail,
}) })
timing.AfterSendAt = time.Now() timing.AfterSendAt = time.Now()
if err := appendNotificationTimingLog(svc.Store().Paths.LogsDir, timing); err != nil { if err := appendNotificationTimingLog(svc.Paths().LogsDir, timing); err != nil {
fyne.LogError("Failed to write notification timing log", err) fyne.LogError("Failed to write notification timing log", err)
} }
}) })
} }
} case app.ErrorOccurred:
if isError { events.add(newEvent(0, "Service", "Error", e.Err.Error()))
events.add(newEvent(0, "Service", "Error", errOccurred.Err.Error())) case app.JobsLoaded:
}
if isJobsLoaded {
// Selecting an existing jobs file replaces the job list without a // Selecting an existing jobs file replaces the job list without a
// prompt, so History carries the receipt: how many jobs, from where. // prompt, so History carries the receipt: how many jobs, from where.
detail := strconv.Itoa(jobsLoaded.Count) + " jobs from " + jobsLoaded.Path detail := strconv.Itoa(e.Count) + " jobs from " + e.Path
events.add(newEvent(0, "Service", "Jobs loaded", detail)) events.add(newEvent(0, "Service", "Jobs loaded", detail))
} }
refresh() refresh()
}) })
})) }))
// Installed after Subscribe so a failure reaches History through
// ErrorOccurred instead of being emitted to no listener.
svc.InstallDesktopIcon(appID, assets.IconBytes())
svc.Start() svc.Start()
tabs := container.NewAppTabs( tabs := container.NewAppTabs(
container.NewTabItemWithIcon("Jobs", theme.ListIcon(), jobsPanel), container.NewTabItemWithIcon("Jobs", theme.ListIcon(), jobsPanel),
container.NewTabItemWithIcon("History", theme.HistoryIcon(), history), container.NewTabItemWithIcon("History", theme.HistoryIcon(), history),
container.NewTabItemWithIcon("Settings", theme.SettingsIcon(), settingsView(w, svc)), container.NewTabItemWithIcon("Settings", theme.SettingsIcon(), settingsView(w, svc, tray)),
) )
tabs.SetTabLocation(container.TabLocationTop) tabs.SetTabLocation(container.TabLocationTop)
+2 -2
View File
@@ -66,7 +66,7 @@ func TestMainViewFitsTheDefaultWindowSize(t *testing.T) {
svc := app.NewService(store, nil) svc := app.NewService(store, nil)
defer svc.Stop() defer svc.Stop()
content, _ := newMainView(w, svc) content, _ := newMainView(w, svc, &trayState{})
min := content.MinSize() min := content.MinSize()
if min.Width > defaultWindowWidth || min.Height > defaultWindowHeight { if min.Width > defaultWindowWidth || min.Height > defaultWindowHeight {
t.Errorf("content.MinSize() = %v, want within %vx%v", min, defaultWindowWidth, defaultWindowHeight) t.Errorf("content.MinSize() = %v, want within %vx%v", min, defaultWindowWidth, defaultWindowHeight)
@@ -111,7 +111,7 @@ func TestMainViewRecordStartupAddsHistoryRow(t *testing.T) {
svc := newTestService(t) svc := newTestService(t)
defer svc.Stop() defer svc.Stop()
content, recordStartup := newMainView(w, svc) content, recordStartup := newMainView(w, svc, &trayState{})
w.SetContent(content) w.SetContent(content)
table := historyTable(t, content) table := historyTable(t, content)
+4 -1
View File
@@ -7,7 +7,10 @@ import (
"time" "time"
) )
const notificationTimingLogName = "notify-timing.log" // notificationTimingLogName deliberately does not end in .log: runner.CleanupLogs
// only manages .log files in the logs directory, and this diagnostic file
// should not be subject to (or counted against) that retention policy.
const notificationTimingLogName = "notify-timing.tsv"
// notificationTiming captures wall-clock points from a failed run through // notificationTiming captures wall-clock points from a failed run through
// SendNotification. It does not include OS toast display latency — Fyne on // SendNotification. It does not include OS toast display latency — Fyne on
+12 -12
View File
@@ -17,9 +17,10 @@ import (
const appID = "ru.mixeme.gosentry.desktop" const appID = "ru.mixeme.gosentry.desktop"
// defaultWindowWidth and defaultWindowHeight are the size the window opens at // defaultWindowWidth and defaultWindowHeight are the size the window opens at
// on first launch (later launches restore the last size from preferences). // on every launch. Window size persistence is frozen (see ROADMAP.md), so
// Fyne enforces the assembled content's MinSize as a hard floor over these, so // there is no saved size to restore. Fyne enforces the assembled content's
// they only take effect if the content actually fits within them. // MinSize as a hard floor over these, so they only take effect if the content
// actually fits within them.
const defaultWindowWidth = 1024 const defaultWindowWidth = 1024
const defaultWindowHeight = 660 const defaultWindowHeight = 660
@@ -60,25 +61,24 @@ func Run(startInTray bool) {
w := a.NewWindow("GoSentry " + app.Version) w := a.NewWindow("GoSentry " + app.Version)
setWindowsNotificationIcon() setWindowsNotificationIcon()
prefs := a.Preferences() w.Resize(fyne.NewSize(defaultWindowWidth, defaultWindowHeight))
winW := float32(prefs.FloatWithFallback("window.width", defaultWindowWidth))
winH := float32(prefs.FloatWithFallback("window.height", defaultWindowHeight))
w.Resize(fyne.NewSize(winW, winH))
svc, err := app.Open() svc, err := app.Open()
if err != nil { if err != nil {
w.SetContent(container.NewPadded(widget.NewLabel("Failed to load GoSentry configuration: " + err.Error()))) w.SetContent(container.NewPadded(widget.NewLabel("Failed to load GoSentry configuration: " + err.Error())))
a.Run() a.Run()
return return
} }
keepInTray = svc.Store().Config.KeepRunningInTray config := svc.Config()
keepInTray = config.KeepRunningInTray
startHidden = resolveStartHidden(startInTray, keepInTray) startHidden = resolveStartHidden(startInTray, keepInTray)
applyTrayBehavior(a, w, keepInTray, false) tray := &trayState{}
tray.apply(a, w, keepInTray, false)
// Apply the persisted theme before building content so the window renders in // Apply the persisted theme before building content so the window renders in
// the chosen theme from the first frame rather than flashing the default one. // the chosen theme from the first frame rather than flashing the default one.
applyTheme(a, svc.Store().Config.Theme) applyTheme(a, config.Theme)
content, recordStartup := newMainView(w, svc) content, recordStartup := newMainView(w, svc, tray)
w.SetContent(content) w.SetContent(content)
serveSingleInstance(instanceListener, w) serveSingleInstance(instanceListener, w, tray)
if startHidden { if startHidden {
// Autostart launches intentionally stay hidden, so "window shown" would be // Autostart launches intentionally stay hidden, so "window shown" would be
// a misleading metric. Record a separate startup event for the tray path // a misleading metric. Record a separate startup event for the tray path
+67 -45
View File
@@ -26,8 +26,15 @@ var settingsCaptions = []string{
"GoSentry", "Go", "Fyne", "Repository", "GoSentry", "Go", "Fyne", "Repository",
} }
func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { func settingsView(w fyne.Window, svc *app.Service, tray *trayState) fyne.CanvasObject {
store := svc.Store() // saved mirrors the config as last persisted (or freshly loaded at
// construction); it is a local copy the closures below compare the form
// against and reassign after a successful save, rather than holding onto
// the live *storage.Store the Service owns (see app.Service.Config).
// paths never changes after construction of this view — AppDir and
// ConfigPath are fixed for the process — so it is read once, not refreshed.
saved := svc.Config()
paths := svc.Paths()
// updateSaveState compares the form to the saved config and enables Save only // updateSaveState compares the form to the saved config and enables Save only
// when something differs. It is defined below (once Save and every field // when something differs. It is defined below (once Save and every field
// exist) but declared here so the field change handlers can reference it. // exist) but declared here so the field change handlers can reference it.
@@ -36,23 +43,42 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
// both the initial load and the Cancel/Defaults buttons below. // both the initial load and the Cancel/Defaults buttons below.
var loadFields func(domain.Config) var loadFields func(domain.Config)
startOnLogin := widget.NewCheck("Start on login", nil) startOnLogin := widget.NewCheck("Start on login", nil)
startOnLogin.SetChecked(store.Config.StartOnLogin) startOnLogin.SetChecked(saved.StartOnLogin)
minimizeToTray := widget.NewCheck("Keep running in the system tray", nil) minimizeToTray := widget.NewCheck("Keep running in the system tray", nil)
minimizeToTray.SetChecked(store.Config.KeepRunningInTray) minimizeToTray.SetChecked(saved.KeepRunningInTray)
autostartStatus := widget.NewLabel("") autostartStatus := widget.NewLabel("")
trayRestartHint := widget.NewLabel("") trayRestartHint := widget.NewLabel("")
trayRestartHint.Truncation = fyne.TextTruncateClip trayRestartHint.Truncation = fyne.TextTruncateClip
// autostartCheckGen guards against an in-flight check's result landing after
// a newer one started (e.g. the user toggles a checkbox again before the
// first check's PowerShell call returns). Both the increment and the compare
// happen on the main/Fyne thread, so this needs no lock of its own.
var autostartCheckGen int
refreshAutostartStatus := func() { refreshAutostartStatus := func() {
if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) { if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) {
autostartStatus.SetText("Pending: save settings to apply") autostartStatus.SetText("Pending: save settings to apply")
return return
} }
// svc.AutostartStatus() reaches readShortcut on Windows, which spawns
// powershell.exe and blocks on CombinedOutput() — hundreds of milliseconds
// of cold start. Running it off the main thread keeps that from freezing
// the window on construction and on every checkbox toggle.
autostartStatus.SetText("Checking...")
autostartCheckGen++
gen := autostartCheckGen
go func() {
ok, message := svc.AutostartStatus() ok, message := svc.AutostartStatus()
fyne.Do(func() {
if gen != autostartCheckGen {
return
}
if ok { if ok {
autostartStatus.SetText("OK: " + message) autostartStatus.SetText("OK: " + message)
return return
} }
autostartStatus.SetText("Problem: " + message) autostartStatus.SetText("Problem: " + message)
})
}()
} }
refreshTrayRestartHint := func(pending bool) { refreshTrayRestartHint := func(pending bool) {
if pending { if pending {
@@ -67,15 +93,15 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
} }
minimizeToTray.OnChanged = func(bool) { minimizeToTray.OnChanged = func(bool) {
refreshAutostartStatus() refreshAutostartStatus()
refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray) refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray)
updateSaveState() updateSaveState()
} }
refreshAutostartStatus() refreshAutostartStatus()
notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil) notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil)
notifications.SetChecked(store.Config.NotifyOnFailure) notifications.SetChecked(saved.NotifyOnFailure)
notifications.OnChanged = func(bool) { updateSaveState() } notifications.OnChanged = func(bool) { updateSaveState() }
themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil) themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil)
themeSelect.SetSelected(themeLabel(store.Config.Theme)) themeSelect.SetSelected(themeLabel(saved.Theme))
// Preview the theme the moment it is picked so the choice is visible before // Preview the theme the moment it is picked so the choice is visible before
// saving; Save persists it. Reverting the selection reverts the preview, and // saving; Save persists it. Reverting the selection reverts the preview, and
// closing without saving falls back to the stored theme on next launch. // closing without saving falls back to the stored theme on next launch.
@@ -87,20 +113,20 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
[]string{string(domain.ExecutionModeParallel), string(domain.ExecutionModeSequential)}, []string{string(domain.ExecutionModeParallel), string(domain.ExecutionModeSequential)},
nil, nil,
) )
executionModeSelect.SetSelected(string(store.Config.ExecutionMode)) executionModeSelect.SetSelected(string(saved.ExecutionMode))
executionModeSelect.OnChanged = func(string) { updateSaveState() } executionModeSelect.OnChanged = func(string) { updateSaveState() }
overlapPolicySelect := widget.NewSelect( overlapPolicySelect := widget.NewSelect(
[]string{string(domain.OverlapPolicySkip), string(domain.OverlapPolicyQueue)}, []string{string(domain.OverlapPolicySkip), string(domain.OverlapPolicyQueue)},
nil, nil,
) )
overlapPolicySelect.SetSelected(string(store.Config.OverlapPolicy)) overlapPolicySelect.SetSelected(string(saved.OverlapPolicy))
overlapPolicySelect.OnChanged = func(string) { updateSaveState() } overlapPolicySelect.OnChanged = func(string) { updateSaveState() }
defaultTimeout := widget.NewEntry() defaultTimeout := widget.NewEntry()
defaultTimeout.SetPlaceHolder("0 = no timeout") defaultTimeout.SetPlaceHolder("0 = no timeout")
defaultTimeout.SetText(strconv.Itoa(store.Config.DefaultTimeoutSeconds)) defaultTimeout.SetText(strconv.Itoa(saved.DefaultTimeoutSeconds))
defaultTimeout.OnChanged = func(string) { updateSaveState() } defaultTimeout.OnChanged = func(string) { updateSaveState() }
jobsFile := widget.NewEntry() jobsFile := widget.NewEntry()
jobsFile.SetText(store.Config.JobsFile) jobsFile.SetText(saved.JobsFile)
jobsFile.OnChanged = func(string) { updateSaveState() } jobsFile.OnChanged = func(string) { updateSaveState() }
// The picker only offers existing files; a jobs file that does not exist yet // The picker only offers existing files; a jobs file that does not exist yet
// is entered by typing its path, which Save then creates. // is entered by typing its path, which Save then creates.
@@ -108,7 +134,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
chooseJSONFile(w, jobsFile) chooseJSONFile(w, jobsFile)
}) })
logsDir := widget.NewEntry() logsDir := widget.NewEntry()
logsDir.SetText(store.Config.LogsDir) logsDir.SetText(saved.LogsDir)
logsDir.OnChanged = func(string) { updateSaveState() } logsDir.OnChanged = func(string) { updateSaveState() }
logsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() { logsDirBrowse := widget.NewButtonWithIcon("Browse", theme.FolderOpenIcon(), func() {
chooseFolder(w, logsDir) chooseFolder(w, logsDir)
@@ -118,13 +144,15 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
// manager. It reveals whatever the field currently holds, so an edit can be // manager. It reveals whatever the field currently holds, so an edit can be
// checked before Save. // checked before Save.
logsDirOpen := widget.NewButtonWithIcon("Open", theme.FolderIcon(), func() { logsDirOpen := widget.NewButtonWithIcon("Open", theme.FolderIcon(), func() {
openFolder(w, settingsFolderPath(store.Paths.AppDir, logsDir.Text)) openFolder(w, settingsFolderPath(paths.AppDir, logsDir.Text))
}) })
maxLogFiles := widget.NewEntry() maxLogFiles := widget.NewEntry()
maxLogFiles.SetText(strconv.Itoa(store.Config.MaxLogFiles)) maxLogFiles.SetPlaceHolder("0 = unlimited")
maxLogFiles.SetText(strconv.Itoa(saved.MaxLogFiles))
maxLogFiles.OnChanged = func(string) { updateSaveState() } maxLogFiles.OnChanged = func(string) { updateSaveState() }
maxLogAgeDays := widget.NewEntry() maxLogAgeDays := widget.NewEntry()
maxLogAgeDays.SetText(strconv.Itoa(store.Config.MaxLogAgeDays)) maxLogAgeDays.SetPlaceHolder("0 = unlimited")
maxLogAgeDays.SetText(strconv.Itoa(saved.MaxLogAgeDays))
maxLogAgeDays.OnChanged = func(string) { updateSaveState() } maxLogAgeDays.OnChanged = func(string) { updateSaveState() }
// Autostart status sits on its own row beneath the checkbox (rather than // Autostart status sits on its own row beneath the checkbox (rather than
// beside it) so the Application section fits within a half-width column. // beside it) so the Application section fits within a half-width column.
@@ -133,33 +161,22 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
settingsStatus := widget.NewLabel("") settingsStatus := widget.NewLabel("")
saveSettings := widget.NewButtonWithIcon("Save settings", theme.DocumentSaveIcon(), func() { saveSettings := widget.NewButtonWithIcon("Save settings", theme.DocumentSaveIcon(), func() {
files, err := strconv.Atoi(strings.TrimSpace(maxLogFiles.Text)) // Only the parse itself happens here: a numeric field has to become an int
if err != nil || files <= 0 { // before it can go into a domain.Config at all. Everything else — required
settingsStatus.SetText("Max log files must be a positive number") // fields, negative numbers, valid enum values — is Service.UpdateSettings'
return // job (see app.validateConfig), so its error is what the user sees rather
} // than a second copy of the same rules with different wording.
days, err := strconv.Atoi(strings.TrimSpace(maxLogAgeDays.Text)) files, filesErr := strconv.Atoi(strings.TrimSpace(maxLogFiles.Text))
if err != nil || days <= 0 { days, daysErr := strconv.Atoi(strings.TrimSpace(maxLogAgeDays.Text))
settingsStatus.SetText("Max log age days must be a positive number") timeout, timeoutErr := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text))
return if filesErr != nil || daysErr != nil || timeoutErr != nil {
} settingsStatus.SetText("Max log files, max log age days, and default timeout must be numbers")
if strings.TrimSpace(jobsFile.Text) == "" {
settingsStatus.SetText("Jobs file is required")
return
}
if strings.TrimSpace(logsDir.Text) == "" {
settingsStatus.SetText("Logs directory is required")
return
}
timeout, err := strconv.Atoi(strings.TrimSpace(defaultTimeout.Text))
if err != nil || timeout < 0 {
settingsStatus.SetText("Default timeout must not be negative (0 = no timeout)")
return return
} }
// Build the new config from the form and hand it to the Service, which // Build the new config from the form and hand it to the Service, which
// validates it, persists config and jobs to the (possibly new) directory, // validates it, persists config and jobs to the (possibly new) directory,
// and runs log cleanup so tightened retention limits take effect at once. // and runs log cleanup so tightened retention limits take effect at once.
config := store.Config config := saved
config.JobsFile = strings.TrimSpace(jobsFile.Text) config.JobsFile = strings.TrimSpace(jobsFile.Text)
config.LogsDir = strings.TrimSpace(logsDir.Text) config.LogsDir = strings.TrimSpace(logsDir.Text)
config.MaxLogFiles = files config.MaxLogFiles = files
@@ -171,18 +188,23 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
config.OverlapPolicy = domain.OverlapPolicy(overlapPolicySelect.Selected) config.OverlapPolicy = domain.OverlapPolicy(overlapPolicySelect.Selected)
config.DefaultTimeoutSeconds = timeout config.DefaultTimeoutSeconds = timeout
config.Theme = themeFromLabel(themeSelect.Selected) config.Theme = themeFromLabel(themeSelect.Selected)
previousKeepInTray := store.Config.KeepRunningInTray previousKeepInTray := saved.KeepRunningInTray
if err := svc.UpdateSettings(config); err != nil { if err := svc.UpdateSettings(config); err != nil {
settingsStatus.SetText("Save failed: " + err.Error()) settingsStatus.SetText("Save failed: " + err.Error())
return return
} }
// UpdateSettings may re-resolve paths (a jobs-file switch adopts a
// different directory), so pick up the fresh copy rather than assuming
// config is exactly what landed.
saved = svc.Config()
paths = svc.Paths()
if err := svc.ApplyAutostart(); err != nil { if err := svc.ApplyAutostart(); err != nil {
refreshAutostartStatus() refreshAutostartStatus()
settingsStatus.SetText("Saved, autostart failed: " + err.Error()) settingsStatus.SetText("Saved, autostart failed: " + err.Error())
return return
} }
refreshAutostartStatus() refreshAutostartStatus()
applyTrayBehavior(fyne.CurrentApp(), w, config.KeepRunningInTray, true) tray.apply(fyne.CurrentApp(), w, config.KeepRunningInTray, true)
if previousKeepInTray != config.KeepRunningInTray { if previousKeepInTray != config.KeepRunningInTray {
trayRestartHint.SetText(trayRestartHintText) trayRestartHint.SetText(trayRestartHintText)
} else { } else {
@@ -198,7 +220,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
// fields compare against their canonical string form; any unparsable text // fields compare against their canonical string form; any unparsable text
// counts as a change so the user can click Save and see the validation error. // counts as a change so the user can click Save and see the validation error.
updateSaveState = func() { updateSaveState = func() {
c := store.Config c := saved
changed := startOnLogin.Checked != c.StartOnLogin || changed := startOnLogin.Checked != c.StartOnLogin ||
minimizeToTray.Checked != c.KeepRunningInTray || minimizeToTray.Checked != c.KeepRunningInTray ||
notifications.Checked != c.NotifyOnFailure || notifications.Checked != c.NotifyOnFailure ||
@@ -235,17 +257,17 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
logsDir.SetText(c.LogsDir) logsDir.SetText(c.LogsDir)
maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles)) maxLogFiles.SetText(strconv.Itoa(c.MaxLogFiles))
maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays)) maxLogAgeDays.SetText(strconv.Itoa(c.MaxLogAgeDays))
if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) { if settingsPendingAutostart(startOnLogin, minimizeToTray, saved) {
autostartStatus.SetText("Pending: save settings to apply") autostartStatus.SetText("Pending: save settings to apply")
} else { } else {
refreshAutostartStatus() refreshAutostartStatus()
} }
refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray) refreshTrayRestartHint(minimizeToTray.Checked != saved.KeepRunningInTray)
settingsStatus.SetText("") settingsStatus.SetText("")
updateSaveState() updateSaveState()
} }
cancelSettings := widget.NewButtonWithIcon("Cancel", theme.CancelIcon(), func() { cancelSettings := widget.NewButtonWithIcon("Cancel", theme.CancelIcon(), func() {
loadFields(store.Config) loadFields(saved)
}) })
restoreDefaults := widget.NewButtonWithIcon("Defaults", theme.MediaReplayIcon(), func() { restoreDefaults := widget.NewButtonWithIcon("Defaults", theme.MediaReplayIcon(), func() {
loadFields(domain.DefaultConfig()) loadFields(domain.DefaultConfig())
@@ -261,7 +283,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
executionModeSelect: executionModeSelect, executionModeSelect: executionModeSelect,
overlapPolicySelect: overlapPolicySelect, overlapPolicySelect: overlapPolicySelect,
defaultTimeout: defaultTimeout, defaultTimeout: defaultTimeout,
configPath: store.Paths.ConfigPath, configPath: paths.ConfigPath,
jobsFile: jobsFile, jobsFile: jobsFile,
jobsFileBrowse: jobsFileBrowse, jobsFileBrowse: jobsFileBrowse,
logsDir: logsDir, logsDir: logsDir,
+6 -3
View File
@@ -34,11 +34,14 @@ func acquireSingleInstance(showExisting bool) (net.Listener, bool) {
// If the port is unavailable but does not answer as GoSentry, continue // If the port is unavailable but does not answer as GoSentry, continue
// startup instead of making the application impossible to open because of an // startup instead of making the application impossible to open because of an
// unrelated local listener. In the normal duplicate-start case the dial above // unrelated local listener. In the normal duplicate-start case the dial above
// succeeds and this process exits after waking the first instance. // succeeds and this process exits after waking the first instance. The
// consequence of this fallback — two schedulers able to run against the same
// jobs.json and logs directory — is recorded in STANDARDS.md alongside the
// unauthenticated nature of this same port.
return nil, true return nil, true
} }
func serveSingleInstance(listener net.Listener, w fyne.Window) { func serveSingleInstance(listener net.Listener, w fyne.Window, tray *trayState) {
if listener == nil { if listener == nil {
return return
} }
@@ -56,7 +59,7 @@ func serveSingleInstance(listener net.Listener, w fyne.Window) {
// Accept runs on its own goroutine, so focusing the window must be // Accept runs on its own goroutine, so focusing the window must be
// marshaled onto the main thread like every other widget update. // marshaled onto the main thread like every other widget update.
fyne.Do(func() { fyne.Do(func() {
mainWindowHidden = false tray.hidden = false
w.Show() w.Show()
w.RequestFocus() w.RequestFocus()
}) })
+28 -25
View File
@@ -10,16 +10,19 @@ import (
fynedesktop "fyne.io/fyne/v2/driver/desktop" fynedesktop "fyne.io/fyne/v2/driver/desktop"
) )
// systemTrayRegistered tracks whether this process registered a tray icon at // trayState tracks the two pieces of tray-related process state that Fyne
// launch. Fyne cannot add or remove the icon mid-session, so toggling // itself does not expose: whether this process has registered the tray icon
// KeepRunningInTray in Settings updates close behavior immediately and shows a // (Fyne cannot add or remove it mid-session, so toggling KeepRunningInTray in
// restart hint for the icon itself. // Settings updates close behavior immediately but shows a restart hint for the
var systemTrayRegistered bool // icon itself) and whether the primary window is currently hidden via the tray
// close intercept (Fyne exposes no Window.Visible API). Run owns one instance
// mainWindowHidden tracks whether the primary window was hidden via the tray // and passes it to every call site of apply — settingsView's Save handler is
// close intercept. Fyne exposes no Window.Visible API, so the flag drives the // the other one — so the coupling between them is explicit instead of hidden
// reveal-on-tray-disable path in applyTrayBehavior. // behind package-level globals that no test can reset.
var mainWindowHidden bool type trayState struct {
registered bool
hidden bool
}
const trayRestartHintText = "Restart GoSentry for the tray icon change to take effect." const trayRestartHintText = "Restart GoSentry for the tray icon change to take effect."
@@ -27,23 +30,23 @@ func resolveStartHidden(cliStartInTray, keepInTray bool) bool {
return domain.ResolveStartHidden(cliStartInTray, keepInTray) return domain.ResolveStartHidden(cliStartInTray, keepInTray)
} }
// applyTrayBehavior configures window close handling for KeepRunningInTray. // apply configures window close handling for KeepRunningInTray. When
// When revealIfHidden is true and the tray is off, a hidden window is shown so // revealIfHidden is true and the tray is off, a hidden window is shown so the
// the user can still reach the app after disabling the tray mid-session. // user can still reach the app after disabling the tray mid-session.
func applyTrayBehavior(a fyne.App, w fyne.Window, keepInTray bool, revealIfHidden bool) { func (t *trayState) apply(a fyne.App, w fyne.Window, keepInTray bool, revealIfHidden bool) {
if keepInTray && !systemTrayRegistered { if keepInTray && !t.registered {
registerSystemTray(a, w) t.registerSystemTray(a, w)
systemTrayRegistered = true t.registered = true
} }
setWindowCloseBehavior(w, keepInTray) t.setWindowCloseBehavior(w, keepInTray)
if !keepInTray && revealIfHidden && mainWindowHidden { if !keepInTray && revealIfHidden && t.hidden {
mainWindowHidden = false t.hidden = false
w.Show() w.Show()
w.RequestFocus() w.RequestFocus()
} }
} }
func registerSystemTray(a fyne.App, w fyne.Window) { func (t *trayState) registerSystemTray(a fyne.App, w fyne.Window) {
desk, ok := a.(fynedesktop.App) desk, ok := a.(fynedesktop.App)
if !ok { if !ok {
// Not every Fyne driver exposes desktop tray features. Returning silently // Not every Fyne driver exposes desktop tray features. Returning silently
@@ -74,7 +77,7 @@ func registerSystemTray(a fyne.App, w fyne.Window) {
quit.IsQuit = true quit.IsQuit = true
menu := fyne.NewMenu("GoSentry", menu := fyne.NewMenu("GoSentry",
fyne.NewMenuItem("Show", func() { fyne.NewMenuItem("Show", func() {
mainWindowHidden = false t.hidden = false
w.Show() w.Show()
w.RequestFocus() w.RequestFocus()
}), }),
@@ -85,17 +88,17 @@ func registerSystemTray(a fyne.App, w fyne.Window) {
desk.SetSystemTrayWindow(w) desk.SetSystemTrayWindow(w)
} }
func setWindowCloseBehavior(w fyne.Window, keepInTray bool) { func (t *trayState) setWindowCloseBehavior(w fyne.Window, keepInTray bool) {
if keepInTray { if keepInTray {
w.SetCloseIntercept(func() { w.SetCloseIntercept(func() {
// Closing hides the window instead of quitting because scheduler tools are // Closing hides the window instead of quitting because scheduler tools are
// expected to keep working in the background. The explicit Quit tray item // expected to keep working in the background. The explicit Quit tray item
// remains the way to stop the process. // remains the way to stop the process.
mainWindowHidden = true t.hidden = true
w.Hide() w.Hide()
}) })
return return
} }
mainWindowHidden = false t.hidden = false
w.SetCloseIntercept(nil) w.SetCloseIntercept(nil)
} }