Compare commits

20 Commits

Author SHA1 Message Date
mix 1242b22e4f fix: Windows command quoting, atomic JSON/log writes, restore dropped test
Implements items 1-3 of the whole-project review's suggested order
(docs/PROJECT_REVIEW_PLAN.md):

- Restore TestJobListViewIsCompact, accidentally dropped by 5b0e6fe;
  drop the redundant TestDefaultConfigUsesDetailedJobList row from
  TESTS.md and document the two other doc gaps the review found.
- Fix quoteLeadingWindowsProgramPath to find the earliest file-extension
  match at a word boundary instead of the first extension in list order,
  so a .bat/.cmd command whose argument ends in .exe no longer has its
  whole command line mistaken for the program path.
- Write gosentry.json, jobs.json, and run log files atomically (temp
  file + rename) so a crash or power loss mid-write can no longer leave
  a truncated file. Wire Service.Stop() into the app shutdown path so
  it actually runs, cancelling the run context for in-flight runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-06 16:53:59 +03:00
mix 89be009040 docs: remove the whole-project review agenda and its references
REVIEW.md and the /review-project command are gone; other docs no
longer point at them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-06 16:38:38 +03:00
mix eaa37a1192 docs: add the whole-project review action plan
Records the findings of a review of the whole codebase against the agenda in
REVIEW.md, with a suggested order and a model recommendation per item.

The project is in good health: the engine layering holds, the Service.mu
locking contract is obeyed, and the UI layout code observes the
measure-don't-hardcode rule with tests to match. The findings concentrate in
three places instead: paths that only surface after the app has run for a
while (unbounded History growth, PendingRuns surviving a pause), durability of
the JSON files (non-atomic writes, no shutdown path), and one Windows quoting
bug reproduced outside the build, where an argument ending in .exe hijacks
program-path detection for a .bat or .cmd job.

Also settles what happened to the two domain tests TESTS.md still documents:
5b0e6fe dropped them while adding two new rows to TESTS.md and leaving the old
rows in place, so the loss was accidental rather than the test-suite review's
doing. Only TestJobListViewIsCompact is worth restoring — the storage default
test already subsumes the other. Neither loss moved coverage; the 84.4% ->
84.0% dip is new 1.0.2 code, PeekKeepRunningInTray at 0%.

Temporary: delete once the items are done or moved to ROADMAP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-06 13:19:45 +03:00
mix c8a4d31441 release: 1.0.2, tray runtime wiring and Windows notification polish
Move post-1.0.1 changes into a 1.0.2 changelog section and restore 1.0.1
to the shipped release notes.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 23:26:39 +03:00
mix 1e3d14bef2 Log failure-notification timing and plan faster Windows toasts.
Append app-side delays to logs/notify-timing.log, add a PowerShell baseline
script (~773 ms), and track native WinRT toasts in ROADMAP.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 23:15:44 +03:00
mix 0aab9d8db6 Register Windows notification icon via metadata after NewWindow.
Fyne toasts read App.Icon without SetIcon, preserving the PE multi-size
window and taskbar icon while giving failure notifications app artwork.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 23:02:52 +03:00
mix 0a50f3c66b Add disabled failure sample job for testing desktop notifications.
Seed Failure notification test in defaultJobs so new installs can verify
Settings notifications via Run now without scheduler spam.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 22:50:02 +03:00
mix 1240297cca docs: document platform layer rationale in ARCHITECTURE
Explain why autostart, file manager, shell, and winproc are OS-specific, where compile-time vs runtime branching applies, and rules for new platform code.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 22:46:48 +03:00
mix 5b0e6fe51b Wire KeepRunningInTray to runtime so tray, close, and autostart follow the saved setting.
Autostart entries pass --start-in-tray only when the tray is enabled; Settings warns that the notification icon needs a restart (Fyne limitation).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 22:44:02 +03:00
mix 5170cc5f99 release: 1.0.1, branded theme default and test-suite review
Move post-v1.0.0 changes out of the 1.0.0 changelog section into 1.0.1,
where they belong: GoSentry theme as default, System label, About GitHub link,
Disable auto inset, README @every docs, and the completed test-suite review.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 22:58:13 +03:00
mix a735bfd116 docs: retire the test review plan, keep its durable findings
Every item in TEST_REVIEW_PLAN.md is done or decided, so the working
document goes as its own header instructed. What outlives it moves to
where a later reviewer will actually look:

- TESTS.md gains the -coverpkg command and the 84.4% baseline (per-package
  figures understate the suite), design principle 9 (redundancy is judged
  by comparing coverage profiles, and identical coverage alone is not
  grounds for deletion), a table of the look-alike tests that are kept
  with the reason each survives, and the list of functions deliberately
  at 0%.
- STANDARDS.md's "Intentional behavior" section points at both lists, so
  the mechanism REVIEW.md describes still reaches them.

Dropped as spent: the per-item checklists, the suggested order, and the
model-selection table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:53:13 +03:00
mix 28f0a0d8e2 test: resolve the thin-test item, decline the runner merge
Item 4 of the test-suite review:

- Delete TestEmitWithNoObserversIsNoop (no assertion; ranging a nil slice
  cannot panic) and TestStoreReturnsWiredStore (a getter returning its
  own field).
- Collapse the four TestFilteredJobIndexes* tests into one table-driven
  TestFilteredJobIndexes, matching TestFilterValue above it.
- Replace the TestMainViewBuilds smoke test with
  TestMainViewRecordStartupAddsHistoryRow, which calls the recordStartup
  closure for both wordings run.go selects between and asserts the rows
  reach the History table through its own cell callbacks. Keeps the
  unique coverage the review identified and adds the !windowShown branch.

Item 5 is declined with measurements: the three RunJob tests cost 0.14 s
combined, so merging them saves ~90 ms while forcing their three
fixtures (including the only Manual trigger) into one. The runner
package's runtime is the two timeout tests, not subprocess spawns.

go vet and go test -race pass for src/app and src/ui.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:46:26 +03:00
mix 2ef18e759c test: delete duplicate-coverage tests, fix TESTS.md drift, drop hand-rolled itoa
Items 1-3 of the 2026-08-04 test-suite review: TestCleanupLogsKeepsFilesWithinAgeLimit,
TestRunDueEmptyOverlapInheritsGlobal, and TestSameWindowsPathHandlesSpaces had
byte-identical coverage to an existing test and no assertion the survivor lacked.
storage.defaultJobs, the one accidental 0% coverage gap the review found, is now
covered and TESTS.md corrected to match. seed_test.go's itoa is replaced with
strconv.FormatInt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-04 22:30:27 +03:00
mix 77bd2db286 docs: recommend a model per item in the test review plan
The deciding factor is the slow feedback loop, not task size: the ui
package needs CGO and the MSYS2 toolchain, and a cold test run took 258s
during the review. Getting an edit right on the first pass is worth more
than generating it faster.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:13:04 +03:00
mix 1b6a3604cf docs: add the test-suite review action plan
Records the findings of a review of the whole test suite: three duplicate
tests confirmed by comparing coverage profiles, two inaccuracies in
TESTS.md, a hand-rolled itoa in seed_test.go, and four tests thin enough
to need a decision.

Also lists the pairs that share a coverage profile but assert different
properties, and the functions whose zero coverage is deliberate, so a
later pass does not re-report them. Temporary: delete once the items are
done or moved to ROADMAP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:11:50 +03:00
mix cb37377346 Add top inset to the Disable auto row in the Jobs sidebar.
The row sat flush against the tab bar while the VBox gap below was already one theme padding; matching that on top balances the spacing.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 22:10:11 +03:00
mix b2402f4c72 Rename the Default theme option to System.
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 21:49:06 +03:00
mix 648325a690 Point the About repository link at GitHub.
The Settings About block now links to mixeme/gosentry instead of the private Gitea mirror.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:35:29 +03:00
mix 27927f3ab1 docs: expand README @every schedule syntax
Document supported Go duration units, combinations, cron alternatives for calendar intervals, the one-second tick floor, and cron descriptors.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:28:42 +03:00
mix 276539c383 Make the branded GoSentry theme the default.
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:18:48 +03:00
47 changed files with 2092 additions and 435 deletions
-26
View File
@@ -1,26 +0,0 @@
---
description: Review the project as a whole against the agenda in docs/REVIEW.md
---
Perform a whole-project review of GoSentry.
Read [docs/REVIEW.md](../../docs/REVIEW.md) first — it is the agenda, and its
nine sections are the areas to cover. Read [docs/STANDARDS.md](../../docs/STANDARDS.md)
and [docs/ARCHITECTURE.md](../../docs/ARCHITECTURE.md) for the rules and
contracts the code is checked against.
$ARGUMENTS narrows the review when given — a package path, a file, or the name
of an agenda section. With no arguments, sweep the whole `src/` tree.
Rules for the report:
- Anything listed under "Intentional behavior" in STANDARDS.md is not a finding.
If you believe such an entry is now wrong, say so explicitly as a challenge to
the decision rather than reporting it as a bug.
- Verify before reporting. Read the surrounding code and, where cheap, confirm
the behavior with a test rather than reasoning about it alone.
- Group findings by agenda section, most severe first, each with the file and
line and what would actually go wrong.
- Report honestly that a section is clean rather than inventing something for it.
- Do not fix anything during the review. Report first; apply fixes only when
asked, following "What happens to the findings" in REVIEW.md.
+2 -7
View File
@@ -8,16 +8,11 @@ application service, scheduler, storage, and command runner in one binary.
- [docs/STANDARDS.md](docs/STANDARDS.md) — **required.** Code-quality rules and - [docs/STANDARDS.md](docs/STANDARDS.md) — **required.** Code-quality rules and
the list of intentional behavior. Do not "fix" anything listed there as the list of intentional behavior. Do not "fix" anything listed there as
intentional; if a change contradicts it, update the document in the same commit. intentional; if a change contradicts it, update the document in the same commit.
- [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) — package contracts and event flow. - [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) — package contracts, event flow, and
§Platform layer (why and where OS-specific code lives).
- [docs/TESTS.md](docs/TESTS.md) — test layout and conventions. - [docs/TESTS.md](docs/TESTS.md) — test layout and conventions.
- [docs/ROADMAP.md](docs/ROADMAP.md) — deliberately out of scope. - [docs/ROADMAP.md](docs/ROADMAP.md) — deliberately out of scope.
## Reviewing the project
When the user asks for a review of the project (rather than of a specific
diff), follow [docs/REVIEW.md](docs/REVIEW.md) — it is the agenda, and the
`/review-project` command runs the same thing. Do not improvise a checklist.
## Key rules (full list in STANDARDS.md) ## Key rules (full list in STANDARDS.md)
- `src/app.Service` is the sole owner of job and runtime state; the UI reads it - `src/app.Service` is the sole owner of job and runtime state; the UI reads it
+47 -11
View File
@@ -33,7 +33,7 @@ creating, grouping, pausing, running, and monitoring scheduled shell commands.
- Desktop notifications on job failure. - Desktop notifications on job failure.
- Windows tray icon: left-click to show the window, right-click for the menu. - Windows tray icon: left-click to show the window, right-click for the menu.
- Autostart on login (Windows shortcut; Linux XDG desktop entry). - Autostart on login (Windows shortcut; Linux XDG desktop entry).
- Detailed or compact job list, and a default or branded theme; both are remembered. - Detailed or compact job list, and a system or branded theme; both are remembered.
## Platforms ## Platforms
@@ -51,7 +51,6 @@ GoSentry is built and tested on **Windows** and **Linux**:
- [Roadmap](docs/ROADMAP.md) — planned work larger than a single bug fix - [Roadmap](docs/ROADMAP.md) — planned work larger than a single bug fix
- [Architecture](docs/ARCHITECTURE.md) — component interaction model - [Architecture](docs/ARCHITECTURE.md) — component interaction model
- [Standards](docs/STANDARDS.md) — quality rules and intentional behavior - [Standards](docs/STANDARDS.md) — quality rules and intentional behavior
- [Review](docs/REVIEW.md) — what a whole-project review looks at
- [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies - [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies
- [Tests](docs/TESTS.md) — test suite layout and how to run it - [Tests](docs/TESTS.md) — test suite layout and how to run it
- [Performance](docs/PERFORMANCE.md) — measured performance findings - [Performance](docs/PERFORMANCE.md) — measured performance findings
@@ -74,7 +73,7 @@ portable application: moving the program folder also moves its configuration.
"execution_mode": "parallel", "execution_mode": "parallel",
"overlap_policy": "skip", "overlap_policy": "skip",
"default_timeout_seconds": 0, "default_timeout_seconds": 0,
"theme": "default", "theme": "gosentry",
"job_list_view": "detailed" "job_list_view": "detailed"
} }
``` ```
@@ -83,7 +82,7 @@ That is the file GoSentry writes on first run. `default_timeout_seconds` is the
run timeout applied to jobs that do not set their own; `0` means no timeout, and run timeout applied to jobs that do not set their own; `0` means no timeout, and
it is written out even though it is zero, because a missing value and a it is written out even though it is zero, because a missing value and a
deliberate "no timeout" have to stay distinguishable in a hand-edited file. deliberate "no timeout" have to stay distinguishable in a hand-edited file.
`theme` is `default` or `gosentry` (the branded teal/amber look), and `theme` is `system` or `gosentry` (the branded teal/amber look), and
`job_list_view` is `detailed` or `compact` — both are remembered from the `job_list_view` is `detailed` or `compact` — both are remembered from the
choices made in the app. Keys left at their off value (`start_on_login`, choices made in the app. Keys left at their off value (`start_on_login`,
`paused`) are omitted until they are turned on. `paused`) are omitted until they are turned on.
@@ -126,22 +125,55 @@ include the run timestamp and job name:
## Schedules ## Schedules
Interval schedules using Go duration syntax: GoSentry accepts two schedule forms: fixed `@every` intervals and standard
5-field cron expressions.
### `@every` intervals
Write `@every` followed by a [Go duration](https://pkg.go.dev/time#ParseDuration)
— a positive number with a unit suffix. Units can be combined in one value:
```text ```text
@every 10s @every 10s every 10 seconds
@every 5m @every 5m every 5 minutes
@every 1h30m @every 1h every hour
@every 1h30m every hour and a half (same as @every 90m)
@every 2h45m10s hours, minutes, and seconds combined
``` ```
Standard 5-field cron expressions: Supported units:
| Unit | Meaning |
|------|---------|
| `ns` | nanoseconds |
| `us`, `µs` | microseconds |
| `ms` | milliseconds |
| `s` | seconds |
| `m` | minutes |
| `h` | hours |
`@every` does **not** support days, weeks, months, or years — those follow a
calendar, not a fixed interval. For “every day at 02:00”, “on the 1st of each
month”, or “once a year”, use a cron expression (below).
The scheduler checks due jobs once per second, so values shorter than `1s` are
accepted but will not fire faster than once a second.
### Cron expressions
Five fields: minute, hour, day-of-month, month, day-of-week.
```text ```text
*/5 * * * * every five minutes */5 * * * * every five minutes
0 2 * * * every day at 02:00 0 2 * * * every day at 02:00
30 9 * * 1-5 weekdays at 09:30 30 9 * * 1-5 weekdays at 09:30
0 0 1 * * first day of every month at midnight
0 0 1 1 * every year on 1 January at midnight
``` ```
Named descriptors are also accepted: `@hourly`, `@daily`, `@weekly`,
`@monthly`, `@yearly` (and `@annually`, `@midnight`).
## Using The App ## Using The App
1. Start GoSentry. 1. Start GoSentry.
@@ -168,8 +200,12 @@ loading a new list discards the run state of the old one.
The **Start on login** checkbox shows an `OK` or `Problem` status. Saving with The **Start on login** checkbox shows an `OK` or `Problem` status. Saving with
it enabled writes an autostart entry using the current executable path. it enabled writes an autostart entry using the current executable path.
Autostart entries include `--start-in-tray` so scheduled jobs run after sign-in When **Keep running in the system tray** is also enabled, the entry includes
without opening the main window. `--start-in-tray` so scheduled jobs run after sign-in without opening the main
window. With the tray option off, autostart still works but opens the main
window normally. Changing the tray setting updates close behaviour and the
autostart entry immediately; the tray icon itself updates only after you restart
GoSentry (a Fyne limitation — see [docs/ROADMAP.md](docs/ROADMAP.md)).
## Queue Settings ## Queue Settings
+2
View File
@@ -44,6 +44,8 @@ import (
// - Tray: SetSystemTrayIcon(IconSmallICO()). The notification area is ICO-native // - Tray: SetSystemTrayIcon(IconSmallICO()). The notification area is ICO-native
// and renders at 16-24px; a single-frame 16x16 .ico pins the hand-tuned glyph // and renders at 16-24px; a single-frame 16x16 .ico pins the hand-tuned glyph
// (a multi-size .ico made the tray pick and downscale a larger frame). // (a multi-size .ico made the tray pick and downscale a larger frame).
// - Desktop toasts: AppMetadata.Icon (set after NewWindow in run.go) feeds
// SendNotification without calling SetIcon, which would override GLFW_ICON.
// //
// Linux / other non-Windows (no PE icon resource exists): // Linux / other non-Windows (no PE icon resource exists):
// - Window titlebar: a.SetIcon(IconSmall()) in run.go feeds the resource to // - Window titlebar: a.SetIcon(IconSmall()) in run.go feeds the resource to
+63 -2
View File
@@ -57,6 +57,65 @@ flowchart LR
svc -->|"Set / Status via Manager"| autostart svc -->|"Set / Status via Manager"| autostart
``` ```
## Platform layer
GoSentry ships one binary per target OS. Platform-specific code is not a
workaround for missing cross-platform support — it **is** the cross-platform
strategy: shared interfaces and call sites, with OS-specific implementations
selected at **compile time** (`*_windows.go`, `//go:build linux`, and similar).
Runtime `runtime.GOOS` checks appear only for small UI details (see below), not
for autostart, file-manager integration, or command invocation.
Callers (`app.Service`, `ui`, `runner`) depend on the shared API; they do not
branch on the operating system.
| Package / file | Windows | Linux | Other (`!windows && !linux`) |
| --- | --- | --- | --- |
| `platform/autostart` | Startup-folder `.lnk` shortcut | XDG `~/.config/autostart/gosentry.desktop` | Stub — `Set` returns an error when enabled |
| `platform/desktop` | no-op | Installs `.desktop` + icon under XDG data home | no-op |
| `platform/filemanager` | `explorer` | `xdg-open` | Unsupported — `Open` returns an error |
| `platform/winproc` | `CREATE_NO_WINDOW` / `HideWindow` on child processes | no-op | no-op |
| `runner/invocation_*` | `cmd.exe /S /C` with Windows-safe quoting | `sh -c` | `sh -c` (same as Linux) |
**Why separate implementations are required**
- **Autostart** — each OS defines its own login startup mechanism (shortcut,
XDG Autostart, LaunchAgents on macOS). There is no portable API in Go, Fyne, or
the standard library; a third-party helper would still wrap the same per-OS
code behind an interface.
- **Opening a folder** — the desktop shell exposes no shared “reveal in file
manager” call; each platform invokes its registered handler (`explorer`,
`xdg-open`, `open` on macOS).
- **Command shell** — users expect OS-native semantics (`cmd.exe` batch files,
`%VAR%`, and path rules on Windows; POSIX `sh` on Linux). A single shell for
all platforms would break commands on one side or the other.
- **Hidden console window** — launching a child process from a GUI app can flash
a console on Windows only; Linux and macOS do not need equivalent flags.
**Deliberate platform choices (not OS API limits)**
- **Window and tray icons** — Fyne accepts icons on every platform, but Windows
renders the notification area and titlebar from multi-size `.ico` resources
(embedded via `packaging/windows/gosentry.rc`), while Linux StatusNotifier
trays scale better from a larger PNG. `ui/run.go` and `ui/tray.go` branch on
`runtime.GOOS` for asset selection only.
- **Sample job commands** in `storage/store.go` — demo `echo` lines differ only
because shell quoting rules differ; real jobs are user-authored per platform.
**Adding new platform code**
- Put OS integration in `src/platform/<name>/` with a small shared API, or use
`*_GOOS.go` files in the owning package when the surface is a single function
(as in `runner/invocation_*`).
- Do not scatter `runtime.GOOS` through `app.Service` or UI business logic.
- Unsupported platforms get an explicit stub (return an error or no-op) rather
than silently doing nothing — see `autostart_other.go` and
`filemanager_other.go`.
macOS autostart and file-manager handlers are not implemented yet; see
[ROADMAP.md](ROADMAP.md) for blocked or deferred cross-platform work (for
example window-maximized detection, which would need per-OS native calls).
## Main Flows ## Main Flows
1. Startup: 1. Startup:
@@ -113,8 +172,10 @@ flowchart LR
7. Autostart: 7. Autostart:
`UpdateSettings` in the Service calls `autostart.Manager.Set`. The Manager `UpdateSettings` in the Service calls `autostart.Manager.Set`. The Manager
interface has two implementations: Windows writes a `.lnk` shortcut to the interface has two implementations: Windows writes a `.lnk` shortcut to the
user Startup folder; Linux writes an XDG Autostart `.desktop` file. Both user Startup folder; Linux writes an XDG Autostart `.desktop` file. When
entries pass `--start-in-tray`. `KeepRunningInTray` is enabled the entry passes `--start-in-tray`; when it is
off the entry launches the executable without that flag so the main window
opens after sign-in.
8. Error surfacing: 8. Error surfacing:
Background errors (failed JSON saves, cleanup errors) are emitted as Background errors (failed JSON saves, cleanup errors) are emitted as
+111
View File
@@ -2,6 +2,117 @@
All notable GoSentry changes are recorded in this file. All notable GoSentry changes are recorded in this file.
## 1.0.2 - 2026-08-05
**KeepRunningInTray is wired to runtime; Windows failure notifications can show
the app icon (experimental).**
**Application:**
- **Keep running in the system tray** now controls behaviour: with the tray on
(default), closing the window hides it and autostart uses `--start-in-tray`;
with the tray off, closing quits the app and autostart opens the main window.
- Saving a tray change updates close behaviour and the autostart entry
immediately. The notification-area icon follows the saved value after a
restart; Settings shows a hint when a restart is needed (Fyne cannot add or
remove the icon mid-session).
- A stale autostart shortcut that still passes `--start-in-tray` no longer hides
the window when the tray setting is off — saved config wins over the CLI flag.
- On Windows, failure toasts can show the app icon: after `NewWindow`,
`AppMetadata.Icon` is registered so Fyne picks up artwork without calling
`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.
**Jobs:**
- New disabled example *Failure notification test* (folder Examples). Run it
manually to trigger a failed run and verify Settings → Notifications without
waiting on the scheduler.
**Documentation:**
- **`docs/ARCHITECTURE.md`** — new §Platform layer: why autostart, file manager,
shell, and winproc are OS-specific; compile-time vs runtime branching; rules
for adding platform code.
**Internal:**
- App-side failure-notification timing is appended to `logs/notify-timing.log`
for diagnosing toast delay (OS latency excluded). `scripts/measure-windows-toast.ps1`
measures the PowerShell baseline on Windows.
## 1.0.1 - 2026-08-04
**The branded theme is the default, Fyne's built-in theme is System, and the
test suite is leaner.**
**Settings:**
- **The branded GoSentry theme is now the default.** Fresh installs, the
**Defaults** button, and configs that omit `theme` all open in the teal/amber
look; users who prefer Fyne's built-in theme can still pick **System** in
Settings.
- The Fyne built-in theme option is labelled **System** (stored as `"system"`);
configs that still say `"default"` are read as System and rewritten on save.
- The **About** repository link points at GitHub (`mixeme/gosentry`) instead of
the private Gitea mirror.
**Jobs:**
- The **Disable auto** row gained a top inset matching the gap below it, so it
no longer sits flush against the tab bar.
**Documentation:**
- The **README Schedules** section now documents `@every` in full: supported Go
duration units (`ns` through `h`), combined values such as `1h30m`, the link to
`time.ParseDuration`, the fact that days/months/years belong in cron rather
than `@every`, the one-second scheduler tick floor, cron examples for monthly
and yearly runs, and the `@hourly`/`@daily`/… descriptors.
**Tests:**
- Three tests with byte-identical coverage to an existing test and no unique
assertion are gone: `TestCleanupLogsKeepsFilesWithinAgeLimit`,
`TestRunDueEmptyOverlapInheritsGlobal` (its one unique setup guard moved into
`TestRunDueQueueRerunsAfterFinish`), and `TestSameWindowsPathHandlesSpaces`
(its spaces case folded into `TestSameWindowsPathIgnoresCaseAndQuotes`'s
fixture). So are two that carried no assertion at all:
`TestEmitWithNoObserversIsNoop` and `TestStoreReturnsWiredStore`.
- The four `TestFilteredJobIndexes*` tests are one table-driven
`TestFilteredJobIndexes`, matching `TestFilterValue` above it.
- `TestMainViewBuilds` is replaced by `TestMainViewRecordStartupAddsHistoryRow`,
which exercises the `recordStartup` closure for both wordings `run.go` selects
between and asserts the rows reach the History table through its cell callbacks,
including the `!windowShown` branch.
- `storage.defaultJobs` — the one accidental 0%-coverage gap the review
found — is now exercised by
`TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun`, which also corrects
`docs/TESTS.md`: `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` never
touched jobs, so the "and a sample job" half of its old description was
wrong.
- `src/runner/seed_test.go`'s hand-rolled `itoa` — 18 lines of digit-by-digit
conversion in a file that already imports `strconv` — is replaced with
`strconv.FormatInt`.
- **`docs/TESTS.md`** records the `-coverpkg` command and the 84.4% baseline
(per-package figures understate the suite), design principle 9 (redundancy is
judged by comparing coverage profiles, and identical coverage alone is not
grounds for deletion), a table of the look-alike tests that are kept with the
reason each survives, and the list of functions deliberately at 0%.
- **`docs/STANDARDS.md`**'s "Intentional behavior" section points at both lists,
so the mechanism `docs/REVIEW.md` describes still reaches them. The spent test
review plan is retired.
## 1.0.0 - 2026-07-27 ## 1.0.0 - 2026-07-27
**The window opens at the size it asks for, and the Jobs divider can be **The window opens at the size it asks for, and the Jobs divider can be
+793
View File
@@ -0,0 +1,793 @@
# 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.
-86
View File
@@ -1,86 +0,0 @@
# GoSentry — Review Agenda
What to look at when reviewing the project as a whole, as opposed to a single
diff. This is the agenda; the rules a review checks against live in
[STANDARDS.md](STANDARDS.md) and [ARCHITECTURE.md](ARCHITECTURE.md).
Scope note: a normal pull-request review checks the change. This agenda is for
a periodic sweep of the whole codebase, so a pass may legitimately end with
"nothing to report" on most items.
## 1. Architecture and project structure
Does the code still match the package map and the event flow in
[ARCHITECTURE.md](ARCHITECTURE.md)? Watch for the boundaries that matter here:
`app.Service` as the sole owner of job and runtime state, the UI reading it
through typed events, `domain` staying free of I/O, and platform-specific code
staying behind the `platform/*` interfaces.
## 2. Complexity against the size of the project
GoSentry is a single-process desktop app with two direct dependencies. Flag
abstraction that is not paying for itself: interfaces with one implementation
and no test seam, indirection added for a use case nobody has asked for, a new
dependency where thirty lines of standard library would do. Also check the
opposite direction — files that have grown past the size guideline in
ARCHITECTURE and should be split the way `jobs_view.go` was.
## 3. Code quality
The checkable rules are in [STANDARDS.md](STANDARDS.md) — error handling, unit
tests for pure helpers, regression tests for fixes, `fyne.Do` for updates off
the main thread. Beyond them: concurrency around `Service.mu`, goroutines whose
lifetime is not obvious, and error paths that report something less useful than
what they caught.
## 4. Documentation and comments
Does every documented behavior still exist, and does every non-obvious behavior
get documented? Check the doc set against the code: README (user-facing
behavior and config keys), ARCHITECTURE (packages and flows), STANDARDS
(rules and intentional behavior), DEVELOPMENT (build), TESTS, PERFORMANCE,
CHANGELOG (an entry per notable change). For comments, the bar is *why*, not
*what* — a comment restating the line below it is noise; an unexplained
workaround is a finding.
## 5. Readability and maintainability
Read a package as someone who has not seen it before. Can the next change be
made without reverse-engineering? Naming that matches the domain vocabulary,
functions that do one thing, and control flow that does not need a diagram.
## 6. Logical errors
Correctness independent of style: scheduling and timing edge cases (overlap
policy, sequential mode, pause interactions), off-by-one and boundary handling,
zero values that mean something (see the timeout rules in STANDARDS), state
that can be observed mid-update, and error paths that leave state inconsistent.
## 7. Legacy code and migrations
The app has no database, so migration means file compatibility: `gosentry.json`
and `jobs.json` written by an older version must keep working. Check that new
`Config` fields are backward compatible, that normalization happens in one
place, and that values which are meaningful zeros are not normalized away. Also
look for code kept alive only for a case that no longer exists.
## 8. Undocumented or under-documented contentious decisions
Any decision a future reader would question needs its reasoning recorded where
it lives: a comment at the code, an entry in the "Intentional behavior" section
of [STANDARDS.md](STANDARDS.md), or — when the work is deferred rather than
decided — a note in [ROADMAP.md](ROADMAP.md), which is where the frozen
window-size work keeps its rationale.
## 9. Other improvement proposals
Anything that does not fit above: build and release ergonomics, test coverage
gaps, dependency health, UX rough edges.
## What happens to the findings
- A defect → fix it, with a regression test when severity is medium or higher.
- Behavior that turns out to be deliberate → record it under "Intentional
behavior" in [STANDARDS.md](STANDARDS.md) so it is not re-reported.
- Work larger than a single fix → [ROADMAP.md](ROADMAP.md), with the reasoning.
- A new rule the review establishes → [STANDARDS.md](STANDARDS.md).
+39 -1
View File
@@ -5,6 +5,44 @@ Completed work is recorded in [CHANGELOG.md](CHANGELOG.md), not here.
## Open Items ## Open Items
### Faster Windows failure notifications
Fyne `SendNotification` on Windows does not call WinRT directly. Each toast
writes a short script to `%TEMP%` and runs it through a **new PowerShell
process** (`app/app_windows.go`), which typically adds **13 seconds** of cold
start before the toast appears. GoSentry's own path from run completion through
`SendNotification` is much smaller and is logged separately.
**Baseline (2026-08-05, `scripts/measure-windows-toast.ps1`, 3 runs on dev
machine):** average **773 ms** per toast (695874 ms), dominated by PowerShell
cold start. Re-run the script when comparing after a native toast implementation.
**App-side timing:** each failure notification appends one line to
`logs/notify-timing.log` (`ms_after_run`, `ms_fyne_do`, `ms_send`,
`ms_app_total`). These columns end when Fyne returns from `SendNotification`; OS
toast latency is not included.
**Direction:** add `src/platform/notify/` with a native Windows toast (WinRT or
a maintained Go wrapper), used for failure notifications on Windows. Keep Fyne
`SendNotification` on Linux (DBus / xdg-desktop-portal) unless profiling shows it
needs the same treatment.
### Dynamic tray icon toggle
Fyne exposes `SetSystemTrayIcon` and related APIs only at application startup.
There is no supported way to register or remove the notification-area icon
after the process is running.
GoSentry now honours `KeepRunningInTray` from config: close behaviour and the
autostart entry update immediately when the user saves Settings; the tray icon
follows the saved value on the next launch. Settings shows a restart hint when
the tray checkbox changes.
Revisit when Fyne adds a documented API for mid-session tray registration, or
when a stable cross-platform approach exists without reaching into driver
internals. Until then, removing the restart hint and applying the icon on save
is blocked.
### Update check from GitHub releases ### Update check from GitHub releases
Releases are published as GitHub Releases (tags like `v0.12.0`, built by Releases are published as GitHub Releases (tags like `v0.12.0`, built by
@@ -98,7 +136,7 @@ were already split once:
| `src/storage/store.go` | 265 | | `src/storage/store.go` | 265 |
This is deliberately deferred to the next whole-project review rather than done This is deliberately deferred to the next whole-project review rather than done
piecemeal: [REVIEW.md](REVIEW.md) already asks item 2 to look for exactly this, piecemeal: a future review already asks item 2 to look for exactly this,
a split touches every reader of the file, and doing all six in one pass keeps a split touches every reader of the file, and doing all six in one pass keeps
the seams consistent instead of settling them six different ways. 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
+15 -2
View File
@@ -1,8 +1,7 @@
# GoSentry — Standards # GoSentry — Standards
Quality rules and intentional behavior for contributors. Package contracts live Quality rules and intentional behavior for contributors. Package contracts live
in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md); in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
what a whole-project review looks at, in [REVIEW.md](REVIEW.md).
## Code quality ## Code quality
@@ -66,6 +65,20 @@ change to their shape has to stay compatible on its own.
- **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).
- Several tests share a coverage profile with another test on purpose, and a few
functions sit at 0% on purpose. Both lists live in
[TESTS.md](TESTS.md) — check them before reporting a test as redundant or a
coverage gap as an oversight.
- **`KeepRunningInTray` controls tray and close behavior.** When enabled (the
default), the app registers a system tray icon at launch, closing the window
hides it, and autostart passes `--start-in-tray`. When disabled, no tray icon
is registered at launch, closing the window quits the app, and autostart opens
the main window. Toggling the setting in Settings updates close behavior and
rewrites the autostart entry immediately; the tray icon itself follows the
saved value only after a restart because Fyne has no API to add or remove it
mid-session (see [ROADMAP.md](ROADMAP.md)).
- **`--start-in-tray` defers to config.** A stale autostart shortcut that still
passes the flag does not hide the window when `KeepRunningInTray` is off.
## Out of scope ## Out of scope
+80 -19
View File
@@ -55,6 +55,17 @@ go test -coverprofile=coverage.out ./src/runner
go tool cover -html=coverage.out go tool cover -html=coverage.out
``` ```
Per-package coverage understates the suite, because several packages are
exercised from another one's tests — `domain.NewRuntime`, for instance, is
covered by the `app` tests. Measure the engine packages together instead:
```bash
go test -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
against before concluding that coverage has slipped.
--- ---
## Test Files Overview ## Test Files Overview
@@ -82,13 +93,13 @@ Tests schedule parsing and validation.
**Package:** `domain` **Package:** `domain`
Tests the normalization rule shared by every consumer of the jobs-list density Tests autostart argument helpers and the jobs-list density normalization rule.
setting.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestAutostartArguments` | Verifies `AutostartArguments` returns `--start-in-tray` when the tray is enabled and an empty string when it is off. |
| `TestResolveStartHidden` | Verifies hidden autostart requires both the CLI flag and `KeepRunningInTray`. |
| `TestJobListViewIsCompact` | Verifies only the exact `"compact"` value selects one-line rows: empty, differently-cased, and unrecognised values all read as detailed. | | `TestJobListViewIsCompact` | Verifies only the exact `"compact"` value selects one-line rows: empty, differently-cased, and unrecognised values all read as detailed. |
| `TestDefaultConfigUsesDetailedJobList` | Verifies `DefaultConfig` selects the detailed job list. |
--- ---
@@ -102,7 +113,6 @@ Tests `Service` construction and the state-accessor contract.
|------|---------| |------|---------|
| `TestNewServiceBuildsRuntimePerJob` | Verifies that `NewService` creates a `JobRuntime` entry for every loaded job. | | `TestNewServiceBuildsRuntimePerJob` | Verifies that `NewService` creates a `JobRuntime` entry for every loaded job. |
| `TestJobsReturnsCopy` | Verifies that `Service.Jobs` returns a defensive copy so callers cannot mutate internal state. | | `TestJobsReturnsCopy` | Verifies that `Service.Jobs` returns a defensive copy so callers cannot mutate internal state. |
| `TestStoreReturnsWiredStore` | Verifies that `Service.Store` returns the injected `storage.Store`. |
--- ---
@@ -175,11 +185,10 @@ and scheduler edge cases using injected `runJob` and `primeDue`.
| `TestRunDueParallelStartsAllDueJobs` | Parallel mode: both due jobs enter the runner before either completes. | | `TestRunDueParallelStartsAllDueJobs` | Parallel mode: both due jobs enter the runner before either completes. |
| `TestRunDueSequentialSerializes` | Sequential mode: job 2 waits until job 1 finishes. | | `TestRunDueSequentialSerializes` | Sequential mode: job 2 waits until job 1 finishes. |
| `TestRunDueSkipDropsOverlap` | Global skip: no second concurrent run, `PendingRuns` stays 0. | | `TestRunDueSkipDropsOverlap` | Global skip: no second concurrent run, `PendingRuns` stays 0. |
| `TestRunDueQueueRerunsAfterFinish` | Queue: one deferred run after an in-flight finish. | | `TestRunDueQueueRerunsAfterFinish` | Queue: one deferred run after an in-flight finish; also covers an empty per-job policy inheriting the global default. |
| `TestRunDueQueueDrainsMultipleOverlaps` | Queue: multiple missed ticks drain as separate runs. | | `TestRunDueQueueDrainsMultipleOverlaps` | Queue: multiple missed ticks drain as separate runs. |
| `TestRunDuePerJobQueueOverridesGlobalSkip` | Per-job `queue` beats global `skip`. | | `TestRunDuePerJobQueueOverridesGlobalSkip` | Per-job `queue` beats global `skip`. |
| `TestRunDuePerJobSkipOverridesGlobalQueue` | Per-job `skip` beats global `queue`. | | `TestRunDuePerJobSkipOverridesGlobalQueue` | Per-job `skip` beats global `queue`. |
| `TestRunDueEmptyOverlapInheritsGlobal` | Empty per-job policy inherits the global default. |
| `TestRunNowSequentialGuard` | Manual run refused while another job runs in sequential mode. | | `TestRunNowSequentialGuard` | Manual run refused while another job runs in sequential mode. |
| `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. | | `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. |
| `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. | | `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. |
@@ -196,7 +205,6 @@ Tests the event-emission and observer-subscription machinery.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestEmitDeliversToAllObserversInOrder` | Verifies that all registered observers receive emitted events in registration order. | | `TestEmitDeliversToAllObserversInOrder` | Verifies that all registered observers receive emitted events in registration order. |
| `TestEmitWithNoObserversIsNoop` | Verifies that emitting an event with no observers does not panic. |
| `TestObserverCanReadServiceState` | Verifies that an observer called by `emit` can safely read Service state (jobs, runtimes). | | `TestObserverCanReadServiceState` | Verifies that an observer called by `emit` can safely read Service state (jobs, runtimes). |
--- ---
@@ -234,9 +242,11 @@ 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. |
| `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults and a sample job. | | `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`. |
| `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. |
| `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. |
| `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. |
@@ -354,8 +364,7 @@ Tests log-file cleanup by age and by count.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestCleanupLogsMissingDirReturnsNil` | Verifies that cleanup returns nil (not an error) when the logs directory does not exist. | | `TestCleanupLogsMissingDirReturnsNil` | Verifies that cleanup returns nil (not an error) when the logs directory does not exist. |
| `TestCleanupLogsRemovesFilesPastMaxAge` | Verifies that `.log` files older than `MaxLogAgeDays` are deleted. | | `TestCleanupLogsRemovesFilesPastMaxAge` | Verifies that `.log` files older than `MaxLogAgeDays` are deleted and files within the limit are retained. |
| `TestCleanupLogsKeepsFilesWithinAgeLimit` | Verifies that `.log` files within the age limit are retained. |
| `TestCleanupLogsByCountDeletesOldest` | Verifies that when file count exceeds `MaxLogFiles`, the oldest files are removed first. | | `TestCleanupLogsByCountDeletesOldest` | Verifies that when file count exceeds `MaxLogFiles`, the oldest files are removed first. |
| `TestCleanupLogsNonLogFilesNotDeleted` | Verifies that non-`.log` files in the logs directory are never deleted by cleanup. | | `TestCleanupLogsNonLogFilesNotDeleted` | Verifies that non-`.log` files in the logs directory are never deleted by cleanup. |
| `TestCleanupLogsSubdirsNotDeleted` | Verifies that subdirectories inside the logs directory are not deleted by cleanup. | | `TestCleanupLogsSubdirsNotDeleted` | Verifies that subdirectories inside the logs directory are not deleted by cleanup. |
@@ -372,13 +381,14 @@ Tests Windows autostart via shortcuts in the Startup folder.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestSameWindowsPathIgnoresCaseAndQuotes` | Verifies that Windows path comparison is case-insensitive and handles quote marks correctly. | | `TestSameWindowsPathIgnoresCaseAndQuotes` | Verifies that Windows path comparison is case-insensitive, handles quote marks, and matches paths containing spaces. |
| `TestSameWindowsPathHandlesSpaces` | Verifies that Windows path comparison matches paths with and without surrounding quotes. |
| `TestSameWindowsPathStripsExtendedLengthPrefix` | Verifies that `\\?\`-prefixed paths are compared correctly after stripping the prefix. | | `TestSameWindowsPathStripsExtendedLengthPrefix` | Verifies that `\\?\`-prefixed paths are compared correctly after stripping the prefix. |
| `TestSameWindowsPathMatchesShortNameViaFilesystem` | Verifies that 8.3 short names are resolved to long names for comparison. | | `TestSameWindowsPathMatchesShortNameViaFilesystem` | Verifies that 8.3 short names are resolved to long names for comparison. |
| `TestStartupShortcutPathUsesUserStartupFolder` | Verifies that the shortcut path resolves into `%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup`. | | `TestStartupShortcutPathUsesUserStartupFolder` | Verifies that the shortcut path resolves into `%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup`. |
| `TestCreateStartupShortcutHandlesCyrillicPath` | Verifies that `.lnk` files are created correctly when the executable path contains Cyrillic characters. | | `TestCreateStartupShortcutHandlesCyrillicPath` | Verifies that `.lnk` files are created correctly when the executable path contains Cyrillic characters. |
| `TestCreateStartupShortcutHandlesSpaces` | Verifies that `.lnk` files are created with correct `TargetPath` and `--start-in-tray` arguments when the path contains spaces. | | `TestCreateStartupShortcutHandlesSpaces` | Verifies that `.lnk` files are created with correct `TargetPath` and `--start-in-tray` arguments when the path contains spaces. |
| `TestCreateStartupShortcutWithoutTrayFlag` | Verifies that autostart shortcuts omit `--start-in-tray` when the tray setting is off. |
| `TestAutostartStatusRequiresMatchingTrayFlag` | Verifies `AutostartStatus` reports a problem when the shortcut arguments do not match `KeepRunningInTray`. |
--- ---
@@ -392,6 +402,19 @@ Tests Linux autostart via XDG Desktop Entry files.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestLinuxAutostartStartsInTray` | Verifies that the XDG Desktop Entry is created with `--start-in-tray` in the `Exec=` field. | | `TestLinuxAutostartStartsInTray` | Verifies that the XDG Desktop Entry is created with `--start-in-tray` in the `Exec=` field. |
| `TestLinuxAutostartWithoutTrayFlag` | Verifies that the desktop entry omits `--start-in-tray` when the tray setting is off. |
---
### src/ui/tray_test.go
**Package:** `ui`
Tests startup helpers for tray and autostart interaction.
| Test | Purpose |
|------|---------|
| `TestResolveStartHiddenUsesDomainHelper` | Verifies the UI startup helper stays aligned with `domain.ResolveStartHidden`. |
--- ---
@@ -437,10 +460,7 @@ widgets are assembled.
| `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. | | `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. |
| `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. | | `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. |
| `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. | | `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. |
| `TestFilteredJobIndexesAll` | Verifies that the "All" filter returns indexes for every job. | | `TestFilteredJobIndexes` | Table: verifies the "All" filter returns every index, a named folder returns only its own jobs, "No folder" matches empty and blank folder fields, and an empty job list yields no indexes. |
| `TestFilteredJobIndexesByNamedFolder` | Verifies that filtering by a named folder returns only jobs in that folder. |
| `TestFilteredJobIndexesNoFolder` | Verifies that the "No folder" filter returns only jobs with an empty folder field. |
| `TestFilteredJobIndexesEmptySlice` | Verifies that filtering an empty job slice returns an empty index list. |
| `TestNextJobListViewFlipsBothWays` | Verifies the density toggle alternates between detailed and compact from either starting value. | | `TestNextJobListViewFlipsBothWays` | Verifies the density toggle alternates between detailed and compact from either starting value. |
| `TestViewToggleTextNamesTheAction` | Verifies the toggle button is labelled with the action it performs, not the state it is in. | | `TestViewToggleTextNamesTheAction` | Verifies the toggle button is labelled with the action it performs, not the state it is in. |
| `TestJobListViewToggleShrinksRowsAndPersists` | End-to-end: one tap shrinks the row height, relabels the button, and reaches the config; tapping back undoes all three. | | `TestJobListViewToggleShrinksRowsAndPersists` | End-to-end: one tap shrinks the row height, relabels the button, and reaches the config; tapping back undoes all three. |
@@ -500,6 +520,7 @@ Tests the theme-derived sizing helpers in `layout.go`.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestRowOverlapMatchesInnerPadding` | Pins `rowOverlap` to `-theme.InnerPadding()` under two themes, the property that lets it follow a theme instead of drifting from a hand-tuned literal. | | `TestRowOverlapMatchesInnerPadding` | Pins `rowOverlap` to `-theme.InnerPadding()` under two themes, the property that lets it follow a theme instead of drifting from a hand-tuned literal. |
| `TestCancelRowOverlapAddsBackOneInnerPadding` | Verifies that `cancelRowOverlap` adds back exactly one inner padding on the top edge only, leaving width and the row below unaffected. |
| `TestCaptionColumnWidth` | Covers no captions, one, and several of varying length, at two text sizes. | | `TestCaptionColumnWidth` | Covers no captions, one, and several of varying length, at two text sizes. |
--- ---
@@ -514,8 +535,8 @@ Tests the branded theme and the stored theme choice.
|------|---------| |------|---------|
| `TestGoSentryThemeBrandColors` | Verifies the brand colors land on the semantically correct `ColorName`s in both the light and dark variants. | | `TestGoSentryThemeBrandColors` | Verifies the brand colors land on the semantically correct `ColorName`s in both the light and dark variants. |
| `TestGoSentryThemeDelegatesUnbrandedColors` | Verifies unbranded color names fall through to the base theme rather than rendering transparent. | | `TestGoSentryThemeDelegatesUnbrandedColors` | Verifies unbranded color names fall through to the base theme rather than rendering transparent. |
| `TestThemeForChoice` | Verifies the GoSentry choice yields the branded primary and every other value — including the empty legacy one — yields the default theme. | | `TestThemeForChoice` | Verifies the GoSentry choice and the empty legacy value yield the branded primary; only the explicit system choice yields Fyne's built-in theme. |
| `TestThemeLabelRoundTrip` | Verifies the dropdown labels round-trip and that the empty value maps to the Default label rather than a blank option. | | `TestThemeLabelRoundTrip` | Verifies the dropdown labels round-trip and that the empty value maps to the GoSentry label rather than a blank option. |
--- ---
@@ -528,7 +549,20 @@ Tests main view construction with an injected `*app.Service`.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestMainViewFitsTheDefaultWindowSize` | Verifies the assembled content's minimum fits the window size the app asks for, so Fyne never silently widens the window past it. The store's config path is deliberately long, since it was the path label that used to grow the Settings tab. | | `TestMainViewFitsTheDefaultWindowSize` | Verifies the assembled content's minimum fits the window size the app asks for, so Fyne never silently widens the window past it. The store's config path is deliberately long, since it was the path label that used to grow the Settings tab. |
| `TestMainViewBuilds` | Verifies `newMainView` assembles tabs without panic using `fyne.io/fyne/v2/test`. | | `TestMainViewRecordStartupAddsHistoryRow` | Verifies the `recordStartup` closure `newMainView` returns appends the startup receipt to History and redraws the table, with the windowed and tray wordings `run.go` selects between. |
---
### src/ui/notify_timing_test.go
**Package:** `ui`
Tests the failure-notification timing diagnostics added in 1.0.2.
| 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`). |
| `TestAppendNotificationTimingLogWritesHeaderAndRow` | Verifies `appendNotificationTimingLog` creates `notify-timing.log` with its header on first write and appends a row containing the job name. |
--- ---
@@ -550,6 +584,23 @@ Tests main view construction with an injected `*app.Service`.
8. **Geometry is measured, not eyeballed** — The `ui` tests that build widgets under `test.NewApp()` assert sizes and offsets, and several re-run under a scaled theme. That is what keeps [STANDARDS.md](STANDARDS.md)'s "measure at build time, never a pixel constant" rule enforceable rather than aspirational. 8. **Geometry is measured, not eyeballed** — The `ui` tests that build widgets under `test.NewApp()` assert sizes and offsets, and several re-run under a scaled theme. That is what keeps [STANDARDS.md](STANDARDS.md)'s "measure at build time, never a pixel constant" rule enforceable rather than aspirational.
9. **Redundancy is measured, not read** — Before deleting a test as a duplicate, run both in isolation with `-coverprofile` and compare the profiles. Identical coverage alone is *not* grounds for deletion: several kept tests hit the same statements while asserting genuinely different properties (see the table below). Deletion requires identical coverage **and** assertions that are a subset of the survivor's.
---
## Look-alike tests that are kept
Every pair here has an identical coverage profile, so a redundancy pass will
flag them again. They were measured under principle 9 and kept because the
assertions differ — not because nobody looked.
| Tests | Why both stay |
|-------|---------------|
| `TestSetGlobalPausePersistsToConfigFile` / `TestSetGlobalPauseUpdatesRuntimesAndEmits` | The first asserts the flag reaches `gosentry.json`, which is what makes the pause survive a restart; the second asserts the in-memory runtimes and the emitted event. |
| `TestRunDueQueueDrainsMultipleOverlaps` / `TestRunDueQueueRerunsAfterFinish` | The first drains three queued occurrences rather than one, so it is the test that would catch a drain loop that fires only once. |
| `TestCreateStartupShortcutHandlesCyrillicPath` / `TestCreateStartupShortcutHandlesSpaces` | Non-ASCII paths and paths with spaces are different real-world failure modes for the WScript.Shell COM call. |
| `TestRunJobLogFileAllHeaders` / `TestRunJobRecordFields` / `TestRunJobWritesLogFile` | Three different subjects: the log file's headers, the returned `RunRecord`'s fields, and the log file's name and directory. The fixtures differ too — only `TestRunJobWritesLogFile` runs the `Manual` trigger. Merging them into one `RunJob` call was measured and declined: it saves ~90 ms (the three cost 0.14 s combined; the `runner` package's seconds are `TestRunJobTimesOut` and `TestRunJobZeroTimeoutMeansNoTimeout`, which wait on purpose) and would drop the `Manual` path from the header assertions. |
--- ---
## Remaining Test Coverage Gaps ## Remaining Test Coverage Gaps
@@ -557,3 +608,13 @@ Tests main view construction with an injected `*app.Service`.
- Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end; the `ui` tests assemble views and measure them, but nothing drives a real window. - Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end; the `ui` tests assemble views and measure them, but nothing drives a real window.
- History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [STANDARDS.md](STANDARDS.md)) - History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [STANDARDS.md](STANDARDS.md))
- Fyne's headless driver cannot report a maximized window, which is why window-size persistence stays frozen in [ROADMAP.md](ROADMAP.md) - Fyne's headless driver cannot report a maximized window, which is why window-size persistence stays frozen in [ROADMAP.md](ROADMAP.md)
### Functions deliberately at 0%
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.
- 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.
- The autostart and desktop-icon wrappers — OS integration, driven only on a real desktop.
- `app.Service.ShouldNotifyOnFailure` — a getter under the mutex.
+39
View File
@@ -0,0 +1,39 @@
# Measures the latency of Fyne's Windows toast path: write a short PowerShell
# script to %TEMP% and run it via PowerShell -ExecutionPolicy Bypass, the same
# approach fyne.io/fyne/v2/app uses in app_windows.go SendNotification.
param(
[int]$Iterations = 3
)
$template = @'
$title = "GoSentry timing test"
$content = "benchmark"
$iconPath = "file:///"
[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null
$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastImageAndText02)
$toastXml = [xml] $template.GetXml()
$toastXml.GetElementsByTagName("text")[0].AppendChild($toastXml.CreateTextNode($title)) > $null
$toastXml.GetElementsByTagName("text")[1].AppendChild($toastXml.CreateTextNode($content)) > $null
$toastXml.GetElementsByTagName("image")[0].SetAttribute("src", $iconPath) > $null
$xml = New-Object Windows.Data.Xml.Dom.XmlDocument
$xml.LoadXml($toastXml.OuterXml)
$toast = [Windows.UI.Notifications.ToastNotification]::new($xml)
[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("ru.mixeme.gosentry.desktop").Show($toast);
'@
Write-Host "Fyne-style Windows toast latency ($Iterations run(s), no icon path):"
$totalMs = 0
for ($i = 1; $i -le $Iterations; $i++) {
$scriptPath = Join-Path $env:TEMP ("fyne-timing-test-$i.ps1")
Set-Content -Path $scriptPath -Value $template -Encoding UTF8
$sw = [System.Diagnostics.Stopwatch]::StartNew()
$launch = "(Get-Content -Encoding UTF8 -Path `"$scriptPath`" -Raw) | Invoke-Expression"
& PowerShell -ExecutionPolicy Bypass -Command $launch | Out-Null
$sw.Stop()
$ms = [int]$sw.ElapsedMilliseconds
$totalMs += $ms
Write-Host (" run {0}: {1} ms" -f $i, $ms)
Remove-Item $scriptPath -ErrorAction SilentlyContinue
}
$avg = [math]::Round($totalMs / [double]$Iterations)
Write-Host (" average: {0} ms" -f $avg)
-6
View File
@@ -33,12 +33,6 @@ func TestEmitDeliversToAllObserversInOrder(t *testing.T) {
} }
} }
func TestEmitWithNoObserversIsNoop(t *testing.T) {
svc := newTestService(nil)
// Must not panic with an empty observer list.
svc.emit(JobChanged{})
}
// Observers may read Service state from within OnEvent without deadlocking, // Observers may read Service state from within OnEvent without deadlocking,
// because emit is called outside the state lock. // because emit is called outside the state lock.
func TestObserverCanReadServiceState(t *testing.T) { func TestObserverCanReadServiceState(t *testing.T) {
+4 -4
View File
@@ -481,10 +481,10 @@ func validateConfig(config domain.Config) error {
if config.DefaultTimeoutSeconds < 0 { if config.DefaultTimeoutSeconds < 0 {
return errors.New("default timeout must not be negative (0 means no timeout)") return errors.New("default timeout must not be negative (0 means no timeout)")
} }
// Empty Theme is accepted and normalized to the default on load, so older // Empty Theme is accepted and normalized to the branded theme on load, so
// configs (and hand-built ones) stay valid without an explicit theme. // older configs (and hand-built ones) stay valid without an explicit theme.
if config.Theme != "" && config.Theme != domain.ThemeDefault && config.Theme != domain.ThemeGoSentry { if config.Theme != "" && config.Theme != domain.ThemeSystem && config.Theme != domain.ThemeGoSentry {
return errors.New("theme must be 'default' or 'gosentry'") return errors.New("theme must be 'system' or 'gosentry'")
} }
return nil return nil
} }
+6 -4
View File
@@ -16,24 +16,26 @@ func (s *Service) InstallDesktopIcon(appID string, iconBytes []byte) {
} }
// AutostartStatus reports whether the platform autostart entry matches the // AutostartStatus reports whether the platform autostart entry matches the
// current StartOnLogin setting in the stored config. // current StartOnLogin and KeepRunningInTray settings in the stored config.
func (s *Service) AutostartStatus() (ok bool, message string) { func (s *Service) AutostartStatus() (ok bool, message string) {
s.mu.Lock() s.mu.Lock()
enabled := s.store.Config.StartOnLogin enabled := s.store.Config.StartOnLogin
startInTray := s.store.Config.KeepRunningInTray
execPath := s.store.Paths.ExecutablePath execPath := s.store.Paths.ExecutablePath
manager := s.manager manager := s.manager
s.mu.Unlock() s.mu.Unlock()
if manager == nil { if manager == nil {
return false, "autostart not available" return false, "autostart not available"
} }
return manager.Status(enabled, execPath) return manager.Status(enabled, startInTray, execPath)
} }
// ApplyAutostart writes or removes the platform autostart entry to match the // ApplyAutostart writes or removes the platform autostart entry to match the
// current StartOnLogin setting in the stored config. Call after UpdateSettings. // current StartOnLogin and KeepRunningInTray settings. Call after UpdateSettings.
func (s *Service) ApplyAutostart() error { func (s *Service) ApplyAutostart() error {
s.mu.Lock() s.mu.Lock()
enabled := s.store.Config.StartOnLogin enabled := s.store.Config.StartOnLogin
startInTray := s.store.Config.KeepRunningInTray
execPath := s.store.Paths.ExecutablePath execPath := s.store.Paths.ExecutablePath
iconPath := s.store.Paths.DesktopIcon iconPath := s.store.Paths.DesktopIcon
manager := s.manager manager := s.manager
@@ -41,5 +43,5 @@ func (s *Service) ApplyAutostart() error {
if manager == nil { if manager == nil {
return nil return nil
} }
return manager.Set(enabled, execPath, iconPath) return manager.Set(enabled, startInTray, execPath, iconPath)
} }
+4 -45
View File
@@ -255,6 +255,10 @@ func TestRunDueQueueRerunsAfterFinish(t *testing.T) {
svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{ svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{
{ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true}, {ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true},
}) })
// Empty per-job OverlapPolicy inherits the global queue policy.
if svc.jobs[0].OverlapPolicy != "" {
t.Fatalf("test setup: job OverlapPolicy = %q, want empty (inherit)", svc.jobs[0].OverlapPolicy)
}
entered := make(chan int, 2) entered := make(chan int, 2)
release := make(chan struct{}) release := make(chan struct{})
@@ -451,51 +455,6 @@ func TestRunDuePerJobSkipOverridesGlobalQueue(t *testing.T) {
} }
} }
// TestRunDueEmptyOverlapInheritsGlobal verifies that a job with no own policy
// inherits the global default: with global "queue" and an empty Job.OverlapPolicy
// the job queues a re-run.
func TestRunDueEmptyOverlapInheritsGlobal(t *testing.T) {
svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{
{ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true},
})
if svc.jobs[0].OverlapPolicy != "" {
t.Fatalf("test setup: job OverlapPolicy = %q, want empty (inherit)", svc.jobs[0].OverlapPolicy)
}
entered := make(chan int, 2)
release := make(chan struct{})
svc.runJob = func(_ context.Context, job *domain.Job, _ string, _ string, _ time.Duration) (domain.RunRecord, error) {
entered <- job.ID
<-release
return domain.RunRecord{Time: "t", JobID: job.ID, JobName: job.Name, State: "Success"}, nil
}
done := completions(svc)
primeDue(t, svc, 1)
svc.RunDue(time.Now())
if id := <-entered; id != 1 {
t.Fatalf("started job = %d, want 1", id)
}
primeDue(t, svc, 1)
svc.RunDue(time.Now())
expectNoEntry(t, entered)
svc.mu.Lock()
pending := svc.runtimes[1].PendingRuns
svc.mu.Unlock()
if pending != 1 {
t.Fatalf("empty per-job policy must inherit global queue, PendingRuns = %d", pending)
}
close(release)
waitRecord(t, done)
if id := <-entered; id != 1 {
t.Fatalf("inherited-queue re-run job = %d, want 1", id)
}
waitRecord(t, done)
}
// TestRunNowSequentialGuard verifies the sequential-mode guard in RunNow: a manual // TestRunNowSequentialGuard verifies the sequential-mode guard in RunNow: a manual
// run is refused while another job is running, and allowed once nothing is. // run is refused while another job is running, and allowed once nothing is.
func TestRunNowSequentialGuard(t *testing.T) { func TestRunNowSequentialGuard(t *testing.T) {
-8
View File
@@ -47,11 +47,3 @@ func TestJobsReturnsCopy(t *testing.T) {
t.Errorf("Service state leaked through Jobs(): name = %q, want %q", again[0].Name, "Original") t.Errorf("Service state leaked through Jobs(): name = %q, want %q", again[0].Name, "Original")
} }
} }
func TestStoreReturnsWiredStore(t *testing.T) {
store := &storage.Store{}
svc := NewService(store, nil)
if svc.Store() != store {
t.Error("Store() did not return the wired store")
}
}
+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.0" var Version = "1.0.2"
+22 -5
View File
@@ -5,6 +5,23 @@ package domain
// launches omit this flag and open the normal window. // launches omit this flag and open the normal window.
const StartInTrayArgument = "--start-in-tray" const StartInTrayArgument = "--start-in-tray"
// AutostartArguments returns the command-line suffix written to a platform
// autostart entry when KeepRunningInTray is enabled. An empty string means the
// app should open its window normally after sign-in.
func AutostartArguments(keepInTray bool) string {
if keepInTray {
return StartInTrayArgument
}
return ""
}
// ResolveStartHidden reports whether an autostart launch should skip showing
// the main window. The CLI flag is ignored when KeepRunningInTray is off so a
// stale shortcut cannot hide the app with no tray icon to restore it.
func ResolveStartHidden(cliStartInTray, keepInTray bool) bool {
return cliStartInTray && keepInTray
}
// ExecutionMode controls whether due jobs run concurrently or one at a time. // ExecutionMode controls whether due jobs run concurrently or one at a time.
type ExecutionMode string type ExecutionMode string
@@ -21,8 +38,8 @@ const (
type Theme string type Theme string
const ( const (
// ThemeDefault keeps Fyne's built-in theme — the original look. // ThemeSystem keeps Fyne's built-in theme, following the platform look.
ThemeDefault Theme = "default" ThemeSystem Theme = "system"
// ThemeGoSentry applies the branded teal/amber theme derived from the logo // ThemeGoSentry applies the branded teal/amber theme derived from the logo
// and app icon. // and app icon.
ThemeGoSentry Theme = "gosentry" ThemeGoSentry Theme = "gosentry"
@@ -87,8 +104,8 @@ type Config struct {
// omitempty would hide a deliberate choice from the hand-editable config. // omitempty would hide a deliberate choice from the hand-editable config.
DefaultTimeoutSeconds int `json:"default_timeout_seconds"` DefaultTimeoutSeconds int `json:"default_timeout_seconds"`
Paused bool `json:"paused,omitempty"` Paused bool `json:"paused,omitempty"`
// Theme selects the visual appearance. Empty is treated as ThemeDefault so // Theme selects the visual appearance. Empty is treated as ThemeGoSentry so
// configs written before this field existed keep the original look. // configs written before this field existed pick up the branded look.
Theme Theme `json:"theme,omitempty"` Theme Theme `json:"theme,omitempty"`
// JobListView selects the Jobs list density. Empty is treated as // JobListView selects the Jobs list density. Empty is treated as
// JobListViewDetailed so configs written before this field existed keep the // JobListViewDetailed so configs written before this field existed keep the
@@ -110,7 +127,7 @@ func DefaultConfig() Config {
NotifyOnFailure: true, NotifyOnFailure: true,
ExecutionMode: ExecutionModeParallel, ExecutionMode: ExecutionModeParallel,
OverlapPolicy: OverlapPolicySkip, OverlapPolicy: OverlapPolicySkip,
Theme: ThemeDefault, Theme: ThemeGoSentry,
JobListView: JobListViewDetailed, JobListView: JobListViewDetailed,
DefaultTimeoutSeconds: 0, DefaultTimeoutSeconds: 0,
} }
+25 -6
View File
@@ -2,6 +2,31 @@ package domain
import "testing" import "testing"
func TestAutostartArguments(t *testing.T) {
if got := AutostartArguments(true); got != StartInTrayArgument {
t.Errorf("AutostartArguments(true) = %q, want %q", got, StartInTrayArgument)
}
if got := AutostartArguments(false); got != "" {
t.Errorf("AutostartArguments(false) = %q, want empty", got)
}
}
func TestResolveStartHidden(t *testing.T) {
cases := []struct {
cli, keep, want bool
}{
{true, true, true},
{true, false, false},
{false, true, false},
{false, false, false},
}
for _, tc := range cases {
if got := ResolveStartHidden(tc.cli, tc.keep); got != tc.want {
t.Errorf("ResolveStartHidden(%v, %v) = %v, want %v", tc.cli, tc.keep, got, tc.want)
}
}
}
// TestJobListViewIsCompact pins the normalization rule: only the exact // TestJobListViewIsCompact pins the normalization rule: only the exact
// "compact" value selects the one-line rows, so empty and unrecognised values // "compact" value selects the one-line rows, so empty and unrecognised values
// (including configs written before the field existed) keep the detailed look. // (including configs written before the field existed) keep the detailed look.
@@ -22,9 +47,3 @@ func TestJobListViewIsCompact(t *testing.T) {
} }
} }
} }
func TestDefaultConfigUsesDetailedJobList(t *testing.T) {
if got := DefaultConfig().JobListView; got != JobListViewDetailed {
t.Errorf("default JobListView = %q, want %q", got, JobListViewDetailed)
}
}
+5 -3
View File
@@ -3,7 +3,9 @@ package autostart
// Manager controls platform autostart for the application. // Manager controls platform autostart for the application.
type Manager interface { type Manager interface {
// Set writes or removes the platform autostart entry to match enabled. // Set writes or removes the platform autostart entry to match enabled.
Set(enabled bool, executablePath, iconPath string) error // When enabled, startInTray selects whether the entry passes --start-in-tray.
// Status reports whether the platform autostart entry matches expectedEnabled. Set(enabled, startInTray bool, executablePath, iconPath string) error
Status(expectedEnabled bool, executablePath string) (ok bool, message string) // Status reports whether the platform autostart entry matches expectedEnabled
// and startInTray.
Status(expectedEnabled, startInTray bool, executablePath string) (ok bool, message string)
} }
+20 -10
View File
@@ -17,17 +17,17 @@ type linuxManager struct{}
// New returns the Linux autostart Manager. // New returns the Linux autostart Manager.
func New() Manager { return linuxManager{} } func New() Manager { return linuxManager{} }
func (linuxManager) Set(enabled bool, executablePath, iconPath string) error { func (linuxManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, executablePath, iconPath) return SetAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (linuxManager) Status(expectedEnabled bool, executablePath string) (bool, string) { func (linuxManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, executablePath) return AutostartStatus(expectedEnabled, startInTray, executablePath)
} }
const autostartDesktopFileName = "gosentry.desktop" const autostartDesktopFileName = "gosentry.desktop"
func SetAutostart(enabled 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
@@ -36,15 +36,19 @@ func SetAutostart(enabled bool, executablePath string, iconPath string) error {
if err := os.MkdirAll(filepath.Dir(desktopPath), 0o755); err != nil { if err := os.MkdirAll(filepath.Dir(desktopPath), 0o755); err != nil {
return err return err
} }
execLine := quoteDesktopExec(executablePath)
if args := domain.AutostartArguments(startInTray); args != "" {
execLine += " " + args
}
desktopFile := fmt.Sprintf(`[Desktop Entry] desktopFile := fmt.Sprintf(`[Desktop Entry]
Type=Application Type=Application
Name=GoSentry Name=GoSentry
Comment=GoSentry desktop scheduler Comment=GoSentry desktop scheduler
Exec=%s %s Exec=%s
%s %s
Terminal=false Terminal=false
X-GNOME-Autostart-enabled=true X-GNOME-Autostart-enabled=true
`, quoteDesktopExec(executablePath), domain.StartInTrayArgument, desktopIconLine(iconPath)) `, execLine, desktopIconLine(iconPath))
return os.WriteFile(desktopPath, []byte(desktopFile), 0o644) return os.WriteFile(desktopPath, []byte(desktopFile), 0o644)
} }
@@ -54,7 +58,7 @@ X-GNOME-Autostart-enabled=true
return nil return nil
} }
func AutostartStatus(expectedEnabled 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"
@@ -70,9 +74,15 @@ func AutostartStatus(expectedEnabled bool, executablePath string) (bool, string)
if readErr != nil { if readErr != nil {
return false, "Autostart desktop entry is missing" return false, "Autostart desktop entry is missing"
} }
expectedExec := "Exec=" + quoteDesktopExec(executablePath) + " " + domain.StartInTrayArgument expectedExec := "Exec=" + quoteDesktopExec(executablePath)
if args := domain.AutostartArguments(startInTray); args != "" {
expectedExec += " " + args
}
if !strings.Contains(string(data), expectedExec) { if !strings.Contains(string(data), expectedExec) {
return false, "Autostart desktop entry points to another executable" if startInTray {
return false, "Autostart desktop entry does not start in tray"
}
return false, "Autostart desktop entry starts in tray while setting is off"
} }
return true, "Autostart is configured" return true, "Autostart is configured"
} }
+27 -1
View File
@@ -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, 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)
} }
@@ -33,3 +33,29 @@ func TestLinuxAutostartStartsInTray(t *testing.T) {
} }
} }
func TestLinuxAutostartWithoutTrayFlag(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
executablePath := "/opt/Go Sentry/gosentry"
if err := SetAutostart(true, false, executablePath, ""); err != nil {
t.Fatalf("enable autostart: %v", err)
}
desktopPath, err := autostartDesktopPath()
if err != nil {
t.Fatalf("resolve desktop path: %v", err)
}
data, err := os.ReadFile(desktopPath)
if err != nil {
t.Fatalf("read desktop entry: %v", err)
}
expectedExec := "Exec=" + quoteDesktopExec(executablePath)
if !strings.Contains(string(data), expectedExec) {
t.Fatalf("desktop entry should not include tray flag: %s", data)
}
if strings.Contains(string(data), domain.StartInTrayArgument) {
t.Fatalf("desktop entry must not pass --start-in-tray: %s", data)
}
}
+6 -6
View File
@@ -9,22 +9,22 @@ type otherManager struct{}
// New returns the stub autostart Manager for unsupported platforms. // New returns the stub autostart Manager for unsupported platforms.
func New() Manager { return otherManager{} } func New() Manager { return otherManager{} }
func (otherManager) Set(enabled bool, executablePath, iconPath string) error { func (otherManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, executablePath, iconPath) return SetAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (otherManager) Status(expectedEnabled bool, executablePath string) (bool, string) { func (otherManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, executablePath) return AutostartStatus(expectedEnabled, startInTray, executablePath)
} }
func SetAutostart(enabled 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, 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"
} }
+15 -11
View File
@@ -16,18 +16,18 @@ type windowsManager struct{}
// New returns the Windows autostart Manager. // New returns the Windows autostart Manager.
func New() Manager { return windowsManager{} } func New() Manager { return windowsManager{} }
func (windowsManager) Set(enabled bool, executablePath, iconPath string) error { func (windowsManager) Set(enabled, startInTray bool, executablePath, iconPath string) error {
return SetAutostart(enabled, executablePath, iconPath) return SetAutostart(enabled, startInTray, executablePath, iconPath)
} }
func (windowsManager) Status(expectedEnabled bool, executablePath string) (bool, string) { func (windowsManager) Status(expectedEnabled, startInTray bool, executablePath string) (bool, string) {
return AutostartStatus(expectedEnabled, executablePath) return AutostartStatus(expectedEnabled, startInTray, executablePath)
} }
const autostartName = "GoSentry" const autostartName = "GoSentry"
const startupShortcutFile = autostartName + ".lnk" const startupShortcutFile = autostartName + ".lnk"
func SetAutostart(enabled 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
@@ -39,12 +39,12 @@ func SetAutostart(enabled bool, executablePath string, iconPath string) error {
} }
if enabled { if enabled {
return createStartupShortcut(shortcutPath, executablePath, iconPath) return createStartupShortcut(shortcutPath, executablePath, iconPath, domain.AutostartArguments(startInTray))
} }
return removeIfExists(shortcutPath) return removeIfExists(shortcutPath)
} }
func AutostartStatus(expectedEnabled 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"
@@ -74,8 +74,12 @@ func AutostartStatus(expectedEnabled bool, executablePath string) (bool, string)
if !sameWindowsPath(actual, executablePath) { if !sameWindowsPath(actual, executablePath) {
return false, "Autostart shortcut points to another executable" return false, "Autostart shortcut points to another executable"
} }
if strings.TrimSpace(arguments) != domain.StartInTrayArgument { expectedArgs := domain.AutostartArguments(startInTray)
return false, "Autostart shortcut does not start in tray" if strings.TrimSpace(arguments) != expectedArgs {
if startInTray {
return false, "Autostart shortcut does not start in tray"
}
return false, "Autostart shortcut starts in tray while setting is off"
} }
return true, "Autostart is configured" return true, "Autostart is configured"
} }
@@ -88,7 +92,7 @@ func startupShortcutPath() (string, error) {
return filepath.Join(appData, "Microsoft", "Windows", "Start Menu", "Programs", "Startup", startupShortcutFile), nil return filepath.Join(appData, "Microsoft", "Windows", "Start Menu", "Programs", "Startup", startupShortcutFile), nil
} }
func createStartupShortcut(shortcutPath string, executablePath string, iconPath string) error { func createStartupShortcut(shortcutPath string, executablePath string, iconPath string, arguments string) error {
if err := os.MkdirAll(filepath.Dir(shortcutPath), 0755); err != nil { if err := os.MkdirAll(filepath.Dir(shortcutPath), 0755); err != nil {
return err return err
} }
@@ -106,7 +110,7 @@ func createStartupShortcut(shortcutPath string, executablePath string, iconPath
command.Env = append(os.Environ(), command.Env = append(os.Environ(),
"GOSENTRY_SHORTCUT_PATH="+shortcutPath, "GOSENTRY_SHORTCUT_PATH="+shortcutPath,
"GOSENTRY_TARGET_PATH="+executablePath, "GOSENTRY_TARGET_PATH="+executablePath,
"GOSENTRY_ARGUMENTS="+domain.StartInTrayArgument, "GOSENTRY_ARGUMENTS="+arguments,
"GOSENTRY_WORKING_DIRECTORY="+workingDirectory, "GOSENTRY_WORKING_DIRECTORY="+workingDirectory,
"GOSENTRY_ICON_PATH="+iconPath, "GOSENTRY_ICON_PATH="+iconPath,
) )
@@ -12,14 +12,8 @@ import (
) )
func TestSameWindowsPathIgnoresCaseAndQuotes(t *testing.T) { func TestSameWindowsPathIgnoresCaseAndQuotes(t *testing.T) {
if !sameWindowsPath(`"D:\Apps\GoSentry\gosentry.exe"`, `d:\apps\gosentry\gosentry.exe`) {
t.Fatal("expected paths to match")
}
}
func TestSameWindowsPathHandlesSpaces(t *testing.T) {
if !sameWindowsPath(`"D:\Local Git\GoSentry\gosentry.exe"`, `d:\local git\gosentry\gosentry.exe`) { if !sameWindowsPath(`"D:\Local Git\GoSentry\gosentry.exe"`, `d:\local git\gosentry\gosentry.exe`) {
t.Fatal("expected paths with spaces to match") t.Fatal("expected paths to match")
} }
} }
@@ -88,7 +82,7 @@ func TestCreateStartupShortcutHandlesCyrillicPath(t *testing.T) {
t.Fatalf("create target file: %v", err) t.Fatalf("create target file: %v", err)
} }
if err := createStartupShortcut(shortcutPath, targetPath, ""); err != nil { if err := createStartupShortcut(shortcutPath, targetPath, "", domain.StartInTrayArgument); err != nil {
t.Fatalf("create shortcut: %v", err) t.Fatalf("create shortcut: %v", err)
} }
@@ -104,6 +98,51 @@ func TestCreateStartupShortcutHandlesCyrillicPath(t *testing.T) {
} }
} }
func TestCreateStartupShortcutWithoutTrayFlag(t *testing.T) {
tempDir := t.TempDir()
shortcutPath := filepath.Join(tempDir, "GoSentry.lnk")
targetPath := filepath.Join(tempDir, "gosentry.exe")
if err := os.WriteFile(targetPath, []byte("test"), 0644); err != nil {
t.Fatalf("create target file: %v", err)
}
if err := createStartupShortcut(shortcutPath, targetPath, "", ""); err != nil {
t.Fatalf("create shortcut: %v", err)
}
_, arguments, err := readShortcut(shortcutPath)
if err != nil {
t.Fatalf("read shortcut: %v", err)
}
if arguments != "" {
t.Fatalf("shortcut arguments mismatch: got %q want empty", arguments)
}
}
func TestAutostartStatusRequiresMatchingTrayFlag(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("APPDATA", tempDir)
shortcutPath, err := startupShortcutPath()
if err != nil {
t.Fatalf("startupShortcutPath: %v", err)
}
targetPath := filepath.Join(tempDir, "gosentry.exe")
if err := os.WriteFile(targetPath, []byte("test"), 0644); err != nil {
t.Fatalf("create target: %v", err)
}
if err := createStartupShortcut(shortcutPath, targetPath, "", domain.StartInTrayArgument); err != nil {
t.Fatalf("create shortcut: %v", err)
}
ok, message := AutostartStatus(true, false, targetPath)
if ok {
t.Fatalf("expected problem when tray flag mismatches, got OK: %s", message)
}
if message != "Autostart shortcut starts in tray while setting is off" {
t.Fatalf("unexpected message: %q", message)
}
}
func TestCreateStartupShortcutHandlesSpaces(t *testing.T) { func TestCreateStartupShortcutHandlesSpaces(t *testing.T) {
tempDir := t.TempDir() tempDir := t.TempDir()
shortcutPath := filepath.Join(tempDir, "GoSentry test.lnk") shortcutPath := filepath.Join(tempDir, "GoSentry test.lnk")
@@ -115,7 +154,7 @@ func TestCreateStartupShortcutHandlesSpaces(t *testing.T) {
t.Fatalf("create target file: %v", err) t.Fatalf("create target file: %v", err)
} }
if err := createStartupShortcut(shortcutPath, targetPath, ""); err != nil { if err := createStartupShortcut(shortcutPath, targetPath, "", domain.StartInTrayArgument); err != nil {
t.Fatalf("create shortcut: %v", err) t.Fatalf("create shortcut: %v", err)
} }
-16
View File
@@ -50,22 +50,6 @@ func TestCleanupLogsRemovesFilesPastMaxAge(t *testing.T) {
} }
} }
func TestCleanupLogsKeepsFilesWithinAgeLimit(t *testing.T) {
dir := t.TempDir()
for i := 1; i <= 3; i++ {
path := writeLogFile(t, dir, fmt.Sprintf("job_%d.log", i))
setModTime(t, path, time.Duration(i)*24*time.Hour)
}
if err := CleanupLogs(dir, 100, 30); err != nil {
t.Fatal(err)
}
entries, _ := os.ReadDir(dir)
if len(entries) != 3 {
t.Errorf("expected 3 files kept within age limit, got %d", len(entries))
}
}
// TestCleanupLogsByCountDeletesOldest verifies the count-based policy: when more // TestCleanupLogsByCountDeletesOldest verifies the count-based policy: when more
// than maxFiles log files exist the oldest (by modification time) are removed. // than maxFiles log files exist the oldest (by modification time) are removed.
// maxAgeDays=0 disables age-based cleanup so the test exercises count only. // maxAgeDays=0 disables age-based cleanup so the test exercises count only.
+37 -10
View File
@@ -6,6 +6,7 @@ import (
"strings" "strings"
"syscall" "syscall"
"unicode" "unicode"
"unicode/utf8"
) )
func shellCommand(ctx context.Context, command string) *exec.Cmd { func shellCommand(ctx context.Context, command string) *exec.Cmd {
@@ -32,19 +33,45 @@ func quoteLeadingWindowsProgramPath(command string) string {
} }
lower := strings.ToLower(trimmed) lower := strings.ToLower(trimmed)
pathEnd := -1
for _, extension := range []string{".exe", ".cmd", ".bat", ".com"} { for _, extension := range []string{".exe", ".cmd", ".bat", ".com"} {
index := strings.Index(lower, extension) end := earliestBoundedExtensionEnd(lower, extension)
if index < 0 { if end >= 0 && (pathEnd < 0 || end < pathEnd) {
continue pathEnd = end
} }
pathEnd := index + len(extension)
programPath := trimmed[:pathEnd]
if !strings.ContainsFunc(programPath, unicode.IsSpace) {
return command
}
return leadingWhitespace + `"` + programPath + `"` + trimmed[pathEnd:]
} }
return command if pathEnd < 0 {
return command
}
programPath := trimmed[:pathEnd]
if !strings.ContainsFunc(programPath, unicode.IsSpace) {
return command
}
return leadingWhitespace + `"` + programPath + `"` + trimmed[pathEnd:]
}
// earliestBoundedExtensionEnd returns the offset just past the first
// occurrence of extension in s that ends at a token boundary (end of string
// or whitespace), or -1 if none does. Scanning left to right and rejecting
// unbounded matches keeps a trailing "...\App.exe" inside an argument, such
// as "run.bat C:\tool.exe", from being mistaken for the program path.
func earliestBoundedExtensionEnd(s, extension string) int {
offset := 0
for {
index := strings.Index(s[offset:], extension)
if index < 0 {
return -1
}
end := offset + index + len(extension)
if end == len(s) {
return end
}
r, _ := utf8.DecodeRuneInString(s[end:])
if unicode.IsSpace(r) {
return end
}
offset += index + 1
}
} }
func startsWithWindowsRootedPath(command string) bool { func startsWithWindowsRootedPath(command string) bool {
+39 -1
View File
@@ -26,12 +26,50 @@ func writeRunLog(logsDir string, job domain.Job, trigger string, state string, d
path := filepath.Join(logsDir, fileName) path := filepath.Join(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 := os.WriteFile(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)
} }
return path, nil return path, nil
} }
// writeFileAtomic writes data to a temp file in dir, then renames it over
// path. Rename is atomic within a volume on both supported platforms, so a
// crash or a killed process mid-write can never leave path holding a
// truncated log file the way a direct os.WriteFile could.
func writeFileAtomic(dir, path string, data []byte, perm os.FileMode) error {
tmp, err := os.CreateTemp(dir, filepath.Base(path)+".tmp*")
if err != nil {
return err
}
tmpPath := tmp.Name()
success := false
defer func() {
if !success {
os.Remove(tmpPath)
}
}()
if _, err := tmp.Write(data); err != nil {
tmp.Close()
return err
}
if err := tmp.Sync(); err != nil {
tmp.Close()
return err
}
if err := tmp.Close(); err != nil {
return err
}
if err := os.Chmod(tmpPath, perm); err != nil {
return err
}
if err := os.Rename(tmpPath, path); err != nil {
return err
}
success = true
return nil
}
func sanitizeFileName(name string) string { func sanitizeFileName(name string) string {
name = strings.TrimSpace(name) name = strings.TrimSpace(name)
if name == "" { if name == "" {
+41
View File
@@ -51,3 +51,44 @@ func TestWindowsShellCommandLineQuotesUnquotedProgramPath(t *testing.T) {
t.Fatalf("expected command line %q, got %q", want, got) t.Fatalf("expected command line %q, got %q", want, got)
} }
} }
// TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension pins the
// fix for the quoting bug found in the whole-project review: the program
// path must end at the *earliest* extension match that sits at a token
// boundary, not the first extension in the .exe/.cmd/.bat/.com list order,
// and not a substring match inside another word.
func TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension(t *testing.T) {
cases := []struct {
name string
command string
want string
}{
{
name: "bat with unquoted argument",
command: `C:\My Tools\run.bat D:\in.txt`,
want: `"C:\My Tools\run.bat" D:\in.txt`,
},
{
name: "bat program with exe argument",
command: `C:\My Tools\run.bat C:\Windows\System32\notepad.exe`,
want: `"C:\My Tools\run.bat" C:\Windows\System32\notepad.exe`,
},
{
name: "cmd program with exe argument",
command: `C:\Program Files\App\deploy.cmd D:\stage\setup.exe`,
want: `"C:\Program Files\App\deploy.cmd" D:\stage\setup.exe`,
},
{
name: "exe substring inside directory name",
command: `C:\dir.exexample\My Tool\run.bat`,
want: `"C:\dir.exexample\My Tool\run.bat"`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := quoteLeadingWindowsProgramPath(tc.command); got != tc.want {
t.Fatalf("quoteLeadingWindowsProgramPath(%q) = %q, want %q", tc.command, got, tc.want)
}
})
}
}
+1 -20
View File
@@ -22,7 +22,7 @@ func writeTestLog(t *testing.T, dir, filename, state string, durationMS int64, j
content.WriteString("\n") content.WriteString("\n")
} }
if durationMS >= 0 { if durationMS >= 0 {
content.WriteString("state: " + state + "\nduration: " + itoa(durationMS) + "\n\n") content.WriteString("state: " + state + "\nduration: " + strconv.FormatInt(durationMS, 10) + "\n\n")
} else { } else {
content.WriteString("state: " + state + "\n\n") content.WriteString("state: " + state + "\n\n")
} }
@@ -31,25 +31,6 @@ func writeTestLog(t *testing.T, dir, filename, state string, durationMS int64, j
} }
} }
func itoa(n int64) string {
if n == 0 {
return "0"
}
neg := n < 0
if neg {
n = -n
}
buf := make([]byte, 0, 20)
for n > 0 {
buf = append([]byte{byte('0' + n%10)}, buf...)
n /= 10
}
if neg {
buf = append([]byte{'-'}, buf...)
}
return string(buf)
}
func TestSeedStatsBasic(t *testing.T) { func TestSeedStatsBasic(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
job := domain.Job{ID: 1, Name: "Build"} job := domain.Job{ID: 1, Name: "Build"}
+80 -8
View File
@@ -16,6 +16,21 @@ type Store struct {
Config domain.Config Config domain.Config
} }
// 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
// default.
func PeekKeepRunningInTray() bool {
paths, err := ResolvePaths()
if err != nil {
return domain.DefaultConfig().KeepRunningInTray
}
config, err := loadOrCreateConfig(paths)
if err != nil {
return domain.DefaultConfig().KeepRunningInTray
}
return config.KeepRunningInTray
}
func OpenStore() (*Store, []domain.Job, error) { func OpenStore() (*Store, []domain.Job, error) {
paths, err := ResolvePaths() paths, err := ResolvePaths()
if err != nil { if err != nil {
@@ -118,7 +133,10 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) {
// the setting impossible to persist. Negative values are rejected by // the setting impossible to persist. Negative values are rejected by
// app.validateConfig before they can be saved. // app.validateConfig before they can be saved.
if config.Theme == "" { if config.Theme == "" {
config.Theme = domain.ThemeDefault config.Theme = domain.ThemeGoSentry
}
if config.Theme == "default" {
config.Theme = domain.ThemeSystem
} }
return config, nil return config, nil
} }
@@ -151,8 +169,9 @@ func loadOrCreateJobs(path string) ([]domain.Job, error) {
if found { if found {
return jobs, nil return jobs, nil
} }
// Seed harmless sample jobs so a new user can immediately see scheduled // Seed sample jobs so a new user can immediately see scheduled and manual
// and manual execution without inventing a command. // execution without inventing a command. The failure sample stays disabled
// so it does not spam notifications; Run now still works for testing.
jobs = defaultJobs() jobs = defaultJobs()
normalizeJobs(jobs) normalizeJobs(jobs)
return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs}) return jobs, writeJSON(path, domain.JobsFile{Jobs: jobs})
@@ -211,7 +230,8 @@ func (s *Store) applyConfigPaths() {
} }
func writeJSON(path string, value any) error { func writeJSON(path string, value any) error {
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0o755); err != nil {
return err return err
} }
data, err := json.MarshalIndent(value, "", " ") data, err := json.MarshalIndent(value, "", " ")
@@ -221,10 +241,47 @@ func writeJSON(path string, value any) error {
// A trailing newline keeps the file friendly to editors and diff tools that // A trailing newline keeps the file friendly to editors and diff tools that
// expect text files to end with one. // expect text files to end with one.
data = append(data, '\n') data = append(data, '\n')
// WriteFile replaces the full file instead of patching it in place. For small return writeFileAtomic(dir, path, data, 0o644)
// JSON files this is simpler and prevents stale keys from older versions from }
// lingering after the schema changes.
return os.WriteFile(path, data, 0o644) // writeFileAtomic writes data to a temp file in dir, syncs it, then renames it
// over path. Rename is atomic within a volume on both supported platforms, so
// a crash, a power loss, or the process being killed mid-write can never leave
// path holding a truncated or empty file the way a direct os.WriteFile could.
func writeFileAtomic(dir, path string, data []byte, perm os.FileMode) error {
tmp, err := os.CreateTemp(dir, filepath.Base(path)+".tmp*")
if err != nil {
return err
}
tmpPath := tmp.Name()
// Any failure past this point must remove the temp file rather than leave
// it behind for the next write to trip over.
success := false
defer func() {
if !success {
os.Remove(tmpPath)
}
}()
if _, err := tmp.Write(data); err != nil {
tmp.Close()
return err
}
if err := tmp.Sync(); err != nil {
tmp.Close()
return err
}
if err := tmp.Close(); err != nil {
return err
}
if err := os.Chmod(tmpPath, perm); err != nil {
return err
}
if err := os.Rename(tmpPath, path); err != nil {
return err
}
success = true
return nil
} }
func defaultJobs() []domain.Job { func defaultJobs() []domain.Job {
@@ -252,9 +309,24 @@ func defaultJobs() []domain.Job {
Command: echoCommand("This paused sample should not run until enabled"), Command: echoCommand("This paused sample should not run until enabled"),
Enabled: false, Enabled: false,
}, },
{
ID: 4,
Name: "Failure notification test",
Folder: "Examples",
Schedule: "@every 1m",
Command: failCommand(),
Enabled: false,
},
} }
} }
func failCommand() string {
if runtime.GOOS == "windows" {
return "exit /b 1"
}
return "exit 1"
}
func echoCommand(message string) string { func echoCommand(message string) string {
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
return "echo " + message return "echo " + message
+106 -2
View File
@@ -171,8 +171,8 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
if got.DefaultTimeoutSeconds != 0 { if got.DefaultTimeoutSeconds != 0 {
t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds) t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds)
} }
if got.Theme != domain.ThemeDefault { if got.Theme != domain.ThemeGoSentry {
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault) t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeGoSentry)
} }
if got.JobListView != domain.JobListViewDetailed { if got.JobListView != domain.JobListViewDetailed {
t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed) t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed)
@@ -183,6 +183,69 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
} }
} }
// TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing
// jobs.json is created with the sample jobs from defaultJobs, so a new user
// sees scheduled and manual execution without inventing a command.
func TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "jobs.json")
got, err := loadOrCreateJobs(path)
if err != nil {
t.Fatal(err)
}
want := defaultJobs()
if len(got) != len(want) {
t.Fatalf("got %d jobs, want %d", len(got), len(want))
}
for i := range want {
if got[i].Name != want[i].Name || got[i].Schedule != want[i].Schedule || got[i].Command != want[i].Command || got[i].Enabled != want[i].Enabled {
t.Errorf("job %d = %+v, want %+v", i, got[i], want[i])
}
}
// The function must have written the seeded jobs to jobs.json.
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("jobs.json should have been created: %v", err)
}
var file domain.JobsFile
if err := json.Unmarshal(data, &file); err != nil {
t.Fatal(err)
}
if len(file.Jobs) != len(want) {
t.Errorf("jobs.json has %d jobs, want %d", len(file.Jobs), len(want))
}
}
// TestLoadOrCreateConfigMigratesLegacyThemeDefault covers a gosentry.json that
// still stores the retired "default" theme value: load normalizes it to system.
func TestLoadOrCreateConfigMigratesLegacyThemeDefault(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
legacy := map[string]any{
"jobs_file": "jobs.json",
"logs_dir": "logs",
"max_log_files": 100,
"max_log_age_days": 30,
"theme": "default",
}
if err := writeJSON(paths.ConfigPath, legacy); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.Theme != domain.ThemeSystem {
t.Errorf("migrated Theme = %q, want %q", got.Theme, domain.ThemeSystem)
}
}
// TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout" // TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout"
// setting against being normalized away when an existing gosentry.json is read // setting against being normalized away when an existing gosentry.json is read
// back. Loading must not treat 0 as a missing value. // back. Loading must not treat 0 as a missing value.
@@ -367,3 +430,44 @@ func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) {
} }
} }
} }
// TestWriteJSONReplacesFileAtomically pins the durability fix: writeJSON must
// never truncate the destination in place. It writes through a temp file and
// renames over the target, so a reader can never observe a partially written
// file, and an existing file survives untouched if the marshal fails first.
func TestWriteJSONReplacesFileAtomically(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "gosentry.json")
original := domain.DefaultConfig()
original.LogsDir = "logs-original"
if err := writeJSON(path, original); err != nil {
t.Fatal(err)
}
updated := domain.DefaultConfig()
updated.LogsDir = "logs-updated"
if err := writeJSON(path, updated); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
var got domain.Config
if err := json.Unmarshal(data, &got); err != nil {
t.Fatal(err)
}
if got.LogsDir != "logs-updated" {
t.Fatalf("LogsDir = %q, want %q", got.LogsDir, "logs-updated")
}
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
if len(entries) != 1 {
t.Fatalf("expected only the final file in %s, got %v", dir, entries)
}
}
+7 -1
View File
@@ -336,7 +336,13 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
}) })
toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer()) toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer())
globalControls := container.NewHBox(stopAllButton, schedulerState, 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.
+27 -32
View File
@@ -56,48 +56,43 @@ func TestFolderOptionsAppendsUniqueFolders(t *testing.T) {
} }
} }
func TestFilteredJobIndexesAll(t *testing.T) { func TestFilteredJobIndexes(t *testing.T) {
jobs := []domain.Job{
{Folder: "Maintenance"},
{Folder: ""},
{Folder: "Reports"},
}
got := filteredJobIndexes(jobs, allFolders)
if len(got) != 3 {
t.Errorf("allFolders filter: got %d indexes, want 3", len(got))
}
}
func TestFilteredJobIndexesByNamedFolder(t *testing.T) {
jobs := []domain.Job{ jobs := []domain.Job{
{Folder: "Maintenance"}, // index 0 {Folder: "Maintenance"}, // index 0
{Folder: ""}, // index 1 {Folder: ""}, // index 1 — no folder
{Folder: "Maintenance"}, // index 2 {Folder: "Maintenance"}, // index 2
{Folder: "Reports"}, // index 3 {Folder: "Reports"}, // index 3
{Folder: " "}, // index 4 — blank reads as no folder
} }
got := filteredJobIndexes(jobs, "Maintenance") cases := []struct {
if len(got) != 2 || got[0] != 0 || got[1] != 2 { name string
t.Errorf("Maintenance filter: got %v, want [0 2]", got) jobs []domain.Job
filter string
want []int
}{
{"all folders", jobs, allFolders, []int{0, 1, 2, 3, 4}},
{"named folder", jobs, "Maintenance", []int{0, 2}},
{"no folder", jobs, noFolder, []int{1, 4}},
{"empty job list", nil, allFolders, nil},
}
for _, tc := range cases {
got := filteredJobIndexes(tc.jobs, tc.filter)
if !sameIndexes(got, tc.want) {
t.Errorf("%s: filteredJobIndexes(_, %q) = %v, want %v", tc.name, tc.filter, got, tc.want)
}
} }
} }
func TestFilteredJobIndexesNoFolder(t *testing.T) { func sameIndexes(got, want []int) bool {
jobs := []domain.Job{ if len(got) != len(want) {
{Folder: "Maintenance"}, // index 0 — excluded return false
{Folder: ""}, // index 1 — no folder → included
{Folder: " "}, // index 2 — blank → included
} }
got := filteredJobIndexes(jobs, noFolder) for i := range got {
if len(got) != 2 || got[0] != 1 || got[1] != 2 { if got[i] != want[i] {
t.Errorf("noFolder filter: got %v, want [1 2]", got) return false
} }
}
func TestFilteredJobIndexesEmptySlice(t *testing.T) {
got := filteredJobIndexes(nil, allFolders)
if len(got) != 0 {
t.Errorf("empty job list should return empty indexes, got %v", got)
} }
return true
} }
func TestNextJobListViewFlipsBothWays(t *testing.T) { func TestNextJobListViewFlipsBothWays(t *testing.T) {
+19 -3
View File
@@ -13,6 +13,8 @@ import (
"fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/theme"
) )
const runRecordTimeLayout = "2006-01-02 15:04:05"
// The UI package aliases domain types to keep widget callbacks short. The actual // The UI package aliases domain types to keep widget callbacks short. The actual
// durable model still lives in src/domain, so UI code does not define a second // durable model still lives in src/domain, so UI code does not define a second
// copy of the scheduler data. // copy of the scheduler data.
@@ -73,9 +75,23 @@ func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time.
if r.State == "Failed" && if r.State == "Failed" &&
(r.Trigger == "Manual" || r.Trigger == "Schedule") && (r.Trigger == "Manual" || r.Trigger == "Schedule") &&
svc.ShouldNotifyOnFailure() { svc.ShouldNotifyOnFailure() {
fyne.CurrentApp().SendNotification(&fyne.Notification{ timing := notificationTiming{
Title: "GoSentry: Job Failed", JobName: r.JobName,
Content: r.JobName + ": " + r.Detail, EmittedAt: time.Now(),
}
if finished, err := time.ParseInLocation(runRecordTimeLayout, r.Time, time.Local); err == nil {
timing.RunFinished = finished
}
fyne.Do(func() {
timing.UIQueuedAt = time.Now()
fyne.CurrentApp().SendNotification(&fyne.Notification{
Title: "GoSentry: Job Failed",
Content: r.JobName + ": " + r.Detail,
})
timing.AfterSendAt = time.Now()
if err := appendNotificationTimingLog(svc.Store().Paths.LogsDir, timing); err != nil {
fyne.LogError("Failed to write notification timing log", err)
}
}) })
} }
} }
+67 -5
View File
@@ -3,12 +3,16 @@ package ui
import ( import (
"path/filepath" "path/filepath"
"testing" "testing"
"time"
"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"
"gitea.mixdep.ru/mix/gosentry/src/storage" "gitea.mixdep.ru/mix/gosentry/src/storage"
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/test" "fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/widget"
) )
// newTestStore builds a Store rooted in a temp directory. It is separate from // newTestStore builds a Store rooted in a temp directory. It is separate from
@@ -69,7 +73,35 @@ func TestMainViewFitsTheDefaultWindowSize(t *testing.T) {
} }
} }
func TestMainViewBuilds(t *testing.T) { // historyTable returns the History tab's table. It is the only widget.Table the
// main view builds, so the search does not need to know the tab order.
func historyTable(t *testing.T, content fyne.CanvasObject) *widget.Table {
t.Helper()
tabs, ok := content.(*container.AppTabs)
if !ok {
t.Fatal("main view is not the expected AppTabs container")
}
for _, item := range tabs.Items {
found := findFirst(item.Content, func(o fyne.CanvasObject) bool {
_, ok := o.(*widget.Table)
return ok
})
if found != nil {
return found.(*widget.Table)
}
}
t.Fatal("main view has no history table")
return nil
}
// TestMainViewRecordStartupAddsHistoryRow covers the second return value of
// newMainView. run.go calls it once per launch with a different windowShown
// flag depending on whether the app started into the tray, and that call is the
// only thing that puts the startup receipt into History — so both the wording
// and the fact that the table is redrawn are worth pinning. Building the full
// tab set and setting it as the window content is a side benefit: no other test
// assembles all three tabs together.
func TestMainViewRecordStartupAddsHistoryRow(t *testing.T) {
testApp := test.NewApp() testApp := test.NewApp()
defer testApp.Quit() defer testApp.Quit()
@@ -80,9 +112,39 @@ func TestMainViewBuilds(t *testing.T) {
defer svc.Stop() defer svc.Stop()
content, recordStartup := newMainView(w, svc) content, recordStartup := newMainView(w, svc)
if content == nil {
t.Fatal("newMainView returned nil content")
}
w.SetContent(content) w.SetContent(content)
recordStartup(0, true)
table := historyTable(t, content)
if rows, _ := table.Length(); rows != 0 {
t.Fatalf("history rows before startup = %d, want 0", rows)
}
recordStartup(1500*time.Millisecond, true)
recordStartup(20*time.Millisecond, false)
rows, _ := table.Length()
if rows != 2 {
t.Fatalf("history rows after two startup records = %d, want 2", rows)
}
// Read the rows back through the table's own cell callbacks, which is what
// the redraw does; a value only in the events slice would not prove the
// table was refreshed with it.
cell := table.CreateCell()
cellText := func(row, col int) string {
table.UpdateCell(widget.TableCellID{Row: row, Col: col}, cell)
return cell.(*widget.Label).Text
}
if got := cellText(0, 2); got != "Application" {
t.Errorf("startup row job = %q, want %q", got, "Application")
}
if got := cellText(0, 3); got != "Started" {
t.Errorf("startup row state = %q, want %q", got, "Started")
}
if got := cellText(0, 4); got != "Window shown in 1.5s" {
t.Errorf("windowed startup detail = %q, want %q", got, "Window shown in 1.5s")
}
if got := cellText(1, 4); got != "Started in tray in 20ms" {
t.Errorf("tray startup detail = %q, want %q", got, "Started in tray in 20ms")
}
} }
+69
View File
@@ -0,0 +1,69 @@
package ui
import (
"fmt"
"os"
"path/filepath"
"time"
)
const notificationTimingLogName = "notify-timing.log"
// notificationTiming captures wall-clock points from a failed run through
// SendNotification. It does not include OS toast display latency — Fyne on
// Windows shows toasts via a separate PowerShell process after SendNotification
// returns.
type notificationTiming struct {
JobName string
RunFinished time.Time
EmittedAt time.Time
UIQueuedAt time.Time
AfterSendAt time.Time
}
func (t notificationTiming) formatLine() string {
return fmt.Sprintf(
"%s\tjob=%s\tms_after_run=%s\tms_fyne_do=%s\tms_send=%s\tms_app_total=%s\n",
t.AfterSendAt.Format(time.RFC3339Nano),
t.JobName,
msBetween(t.RunFinished, t.EmittedAt),
msBetween(t.EmittedAt, t.UIQueuedAt),
msBetween(t.UIQueuedAt, t.AfterSendAt),
msBetween(t.EmittedAt, t.AfterSendAt),
)
}
func msBetween(from, to time.Time) string {
if from.IsZero() || to.IsZero() || to.Before(from) {
return "-"
}
return fmt.Sprintf("%d", to.Sub(from).Milliseconds())
}
func appendNotificationTimingLog(logsDir string, timing notificationTiming) error {
if logsDir == "" {
return nil
}
if err := os.MkdirAll(logsDir, 0o755); err != nil {
return err
}
path := filepath.Join(logsDir, notificationTimingLogName)
file, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return err
}
defer file.Close()
info, err := file.Stat()
if err != nil {
return err
}
if info.Size() == 0 {
if _, err := file.WriteString("# GoSentry failure-notification timing (app side only; OS toast delay is not included)\n" +
"# columns: timestamp job ms_after_run ms_fyne_do ms_send ms_app_total\n"); err != nil {
return err
}
}
_, err = file.WriteString(timing.formatLine())
return err
}
+59
View File
@@ -0,0 +1,59 @@
package ui
import (
"os"
"path/filepath"
"strings"
"testing"
"time"
)
func TestNotificationTimingFormatLine(t *testing.T) {
runFinished := time.Date(2026, 8, 5, 23, 0, 0, 0, time.Local)
emitted := runFinished.Add(15 * time.Millisecond)
uiQueued := emitted.Add(4 * time.Millisecond)
afterSend := uiQueued.Add(2 * time.Millisecond)
line := notificationTiming{
JobName: "Failure notification test",
RunFinished: runFinished,
EmittedAt: emitted,
UIQueuedAt: uiQueued,
AfterSendAt: afterSend,
}.formatLine()
if !strings.Contains(line, "job=Failure notification test") {
t.Fatalf("line = %q, want job name", line)
}
for _, want := range []string{"ms_after_run=15", "ms_fyne_do=4", "ms_send=2", "ms_app_total=6"} {
if !strings.Contains(line, want) {
t.Fatalf("line = %q, want substring %q", line, want)
}
}
}
func TestAppendNotificationTimingLogWritesHeaderAndRow(t *testing.T) {
dir := t.TempDir()
timing := notificationTiming{
JobName: "demo",
RunFinished: time.Now().Add(-10 * time.Millisecond),
EmittedAt: time.Now().Add(-5 * time.Millisecond),
UIQueuedAt: time.Now().Add(-2 * time.Millisecond),
AfterSendAt: time.Now(),
}
if err := appendNotificationTimingLog(dir, timing); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(filepath.Join(dir, notificationTimingLogName))
if err != nil {
t.Fatal(err)
}
text := string(data)
if !strings.HasPrefix(text, "# GoSentry failure-notification timing") {
t.Fatalf("log = %q, want header", text)
}
if !strings.Contains(text, "job=demo") {
t.Fatalf("log = %q, want timing row", text)
}
}
+32 -3
View File
@@ -6,6 +6,7 @@ import (
"gitea.mixdep.ru/mix/gosentry/assets" "gitea.mixdep.ru/mix/gosentry/assets"
"gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/storage"
"fyne.io/fyne/v2" "fyne.io/fyne/v2"
fyneapp "fyne.io/fyne/v2/app" fyneapp "fyne.io/fyne/v2/app"
@@ -29,7 +30,9 @@ const defaultWindowHeight = 660
// mainwindow.go split keeps lifecycle separate from view construction. // mainwindow.go split keeps lifecycle separate from view construction.
func Run(startInTray bool) { func Run(startInTray bool) {
started := time.Now() started := time.Now()
instanceListener, primary := acquireSingleInstance(!startInTray) keepInTray := storage.PeekKeepRunningInTray()
startHidden := resolveStartHidden(startInTray, keepInTray)
instanceListener, primary := acquireSingleInstance(!startHidden)
if !primary { if !primary {
return return
} }
@@ -56,7 +59,7 @@ func Run(startInTray bool) {
} }
w := a.NewWindow("GoSentry " + app.Version) w := a.NewWindow("GoSentry " + app.Version)
configureSystemTray(a, w) setWindowsNotificationIcon()
prefs := a.Preferences() prefs := a.Preferences()
winW := float32(prefs.FloatWithFallback("window.width", defaultWindowWidth)) winW := float32(prefs.FloatWithFallback("window.width", defaultWindowWidth))
winH := float32(prefs.FloatWithFallback("window.height", defaultWindowHeight)) winH := float32(prefs.FloatWithFallback("window.height", defaultWindowHeight))
@@ -67,18 +70,22 @@ func Run(startInTray bool) {
a.Run() a.Run()
return return
} }
keepInTray = svc.Store().Config.KeepRunningInTray
startHidden = resolveStartHidden(startInTray, keepInTray)
applyTrayBehavior(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, svc.Store().Config.Theme)
content, recordStartup := newMainView(w, svc) content, recordStartup := newMainView(w, svc)
w.SetContent(content) w.SetContent(content)
serveSingleInstance(instanceListener, w) serveSingleInstance(instanceListener, w)
if startInTray { 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
// instead of forcing one timing definition onto two different UX flows. // instead of forcing one timing definition onto two different UX flows.
recordStartup(time.Since(started), false) recordStartup(time.Since(started), false)
a.Run() a.Run()
svc.Stop()
return return
} }
// Show the window before recording startup time. Measuring earlier, during // Show the window before recording startup time. Measuring earlier, during
@@ -88,4 +95,26 @@ func Run(startInTray bool) {
w.Show() w.Show()
recordStartup(time.Since(started), true) recordStartup(time.Since(started), true)
a.Run() a.Run()
// a.Run() blocks until the tray's Quit item or a window close calls a.Quit().
// Stopping here — rather than not at all — cancels the run context so an
// in-flight run's os/exec call sees ctx.Done() instead of being orphaned, and
// stops the scheduler goroutine before the process exits.
svc.Stop()
}
// setWindowsNotificationIcon supplies App.Icon for Fyne desktop notifications
// without touching the window or taskbar icon. On Windows those come from the PE
// gosentry.ico resource, so run.go must not call SetIcon. Fyne's NewWindow ends
// with SetIcon(nil), which adopts App.Icon when it is already set — metadata
// must therefore be registered only after the window is created. The tray icon
// is set separately in tray.go via SetSystemTrayIcon.
func setWindowsNotificationIcon() {
if runtime.GOOS != "windows" {
return
}
fyneapp.SetMetadata(fyne.AppMetadata{
ID: appID,
Name: "GoSentry",
Icon: assets.Icon(),
})
} }
+44 -17
View File
@@ -12,7 +12,7 @@ import (
"fyne.io/fyne/v2/widget" "fyne.io/fyne/v2/widget"
) )
const projectRepositoryURL = "https://gitea.mixdep.ru/mix/gosentry" const projectRepositoryURL = "https://github.com/mixeme/gosentry"
// settingsCaptions lists every settingsRow caption in the tab, in no // settingsCaptions lists every settingsRow caption in the tab, in no
// particular order. settingsView measures this once with captionColumnWidth // particular order. settingsView measures this once with captionColumnWidth
@@ -37,8 +37,16 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
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(store.Config.StartOnLogin)
minimizeToTray := widget.NewCheck("Keep running in the system tray", nil)
minimizeToTray.SetChecked(store.Config.KeepRunningInTray)
autostartStatus := widget.NewLabel("") autostartStatus := widget.NewLabel("")
trayRestartHint := widget.NewLabel("")
trayRestartHint.Truncation = fyne.TextTruncateClip
refreshAutostartStatus := func() { refreshAutostartStatus := func() {
if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) {
autostartStatus.SetText("Pending: save settings to apply")
return
}
ok, message := svc.AutostartStatus() ok, message := svc.AutostartStatus()
if ok { if ok {
autostartStatus.SetText("OK: " + message) autostartStatus.SetText("OK: " + message)
@@ -46,22 +54,27 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
} }
autostartStatus.SetText("Problem: " + message) autostartStatus.SetText("Problem: " + message)
} }
startOnLogin.OnChanged = func(bool) { refreshTrayRestartHint := func(pending bool) {
if startOnLogin.Checked != store.Config.StartOnLogin { if pending {
autostartStatus.SetText("Pending: save settings to apply") trayRestartHint.SetText("Pending: restart GoSentry after save for the tray icon change to take effect.")
} else { return
refreshAutostartStatus()
} }
trayRestartHint.SetText("")
}
startOnLogin.OnChanged = func(bool) {
refreshAutostartStatus()
updateSaveState()
}
minimizeToTray.OnChanged = func(bool) {
refreshAutostartStatus()
refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray)
updateSaveState() updateSaveState()
} }
refreshAutostartStatus() refreshAutostartStatus()
minimizeToTray := widget.NewCheck("Keep running in the system tray", nil)
minimizeToTray.SetChecked(store.Config.KeepRunningInTray)
minimizeToTray.OnChanged = func(bool) { updateSaveState() }
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(store.Config.NotifyOnFailure)
notifications.OnChanged = func(bool) { updateSaveState() } notifications.OnChanged = func(bool) { updateSaveState() }
themeSelect := widget.NewSelect([]string{themeLabelDefault, themeLabelGoSentry}, nil) themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil)
themeSelect.SetSelected(themeLabel(store.Config.Theme)) themeSelect.SetSelected(themeLabel(store.Config.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
@@ -158,6 +171,7 @@ 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
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
@@ -168,6 +182,12 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
return return
} }
refreshAutostartStatus() refreshAutostartStatus()
applyTrayBehavior(fyne.CurrentApp(), w, config.KeepRunningInTray, true)
if previousKeepInTray != config.KeepRunningInTray {
trayRestartHint.SetText(trayRestartHintText)
} else {
refreshTrayRestartHint(false)
}
settingsStatus.SetText("Saved") settingsStatus.SetText("Saved")
// The form now matches the persisted config, so disable Save again. // The form now matches the persisted config, so disable Save again.
updateSaveState() updateSaveState()
@@ -215,11 +235,12 @@ 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 startOnLogin.Checked != store.Config.StartOnLogin { if settingsPendingAutostart(startOnLogin, minimizeToTray, store.Config) {
autostartStatus.SetText("Pending: save settings to apply") autostartStatus.SetText("Pending: save settings to apply")
} else { } else {
refreshAutostartStatus() refreshAutostartStatus()
} }
refreshTrayRestartHint(minimizeToTray.Checked != store.Config.KeepRunningInTray)
settingsStatus.SetText("") settingsStatus.SetText("")
updateSaveState() updateSaveState()
} }
@@ -234,6 +255,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
startOnLogin: startOnLogin, startOnLogin: startOnLogin,
autostartStatus: autostartStatus, autostartStatus: autostartStatus,
minimizeToTray: minimizeToTray, minimizeToTray: minimizeToTray,
trayRestartHint: trayRestartHint,
notifications: notifications, notifications: notifications,
themeSelect: themeSelect, themeSelect: themeSelect,
executionModeSelect: executionModeSelect, executionModeSelect: executionModeSelect,
@@ -254,24 +276,29 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
}) })
} }
func settingsPendingAutostart(startOnLogin, minimizeToTray *widget.Check, saved domain.Config) bool {
return startOnLogin.Checked != saved.StartOnLogin ||
minimizeToTray.Checked != saved.KeepRunningInTray
}
// Theme dropdown labels. These are the human-facing captions; themeLabel and // Theme dropdown labels. These are the human-facing captions; themeLabel and
// themeFromLabel translate between them and the stored domain.Theme values so the // themeFromLabel translate between them and the stored domain.Theme values so the
// select never leaks the on-disk "default"/"gosentry" strings to the user. // select never leaks the on-disk "system"/"gosentry" strings to the user.
const ( const (
themeLabelDefault = "Default" themeLabelSystem = "System"
themeLabelGoSentry = "GoSentry" themeLabelGoSentry = "GoSentry"
) )
func themeLabel(choice domain.Theme) string { func themeLabel(choice domain.Theme) string {
if choice == domain.ThemeGoSentry { if choice == domain.ThemeSystem {
return themeLabelGoSentry return themeLabelSystem
} }
return themeLabelDefault return themeLabelGoSentry
} }
func themeFromLabel(label string) domain.Theme { func themeFromLabel(label string) domain.Theme {
if label == themeLabelGoSentry { if label == themeLabelGoSentry {
return domain.ThemeGoSentry return domain.ThemeGoSentry
} }
return domain.ThemeDefault return domain.ThemeSystem
} }
+2
View File
@@ -20,6 +20,7 @@ type settingsFormFields struct {
startOnLogin *widget.Check startOnLogin *widget.Check
autostartStatus *widget.Label autostartStatus *widget.Label
minimizeToTray *widget.Check minimizeToTray *widget.Check
trayRestartHint *widget.Label
notifications *widget.Check notifications *widget.Check
themeSelect *widget.Select themeSelect *widget.Select
executionModeSelect *widget.Select executionModeSelect *widget.Select
@@ -59,6 +60,7 @@ func newSettingsLayout(f settingsFormFields) fyne.CanvasObject {
// empty caption, so the Application section fits in a half-width column. // empty caption, so the Application section fits in a half-width column.
settingsRow(capW, "", f.autostartStatus), settingsRow(capW, "", f.autostartStatus),
settingsRow(capW, "Tray", f.minimizeToTray), settingsRow(capW, "Tray", f.minimizeToTray),
settingsRow(capW, "", f.trayRestartHint),
settingsRow(capW, "Notifications", f.notifications), settingsRow(capW, "Notifications", f.notifications),
// Theme is the one row here whose value is not text: the Select paints // Theme is the one row here whose value is not text: the Select paints
// a box out to the row's edge, so the section's overlap would leave it // a box out to the row's edge, so the section's overlap would leave it
+1
View File
@@ -56,6 +56,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
w.Show() w.Show()
w.RequestFocus() w.RequestFocus()
}) })
+6 -6
View File
@@ -103,14 +103,14 @@ func (t gosentryTheme) Font(style fyne.TextStyle) fyne.Resource { return t.base.
func (t gosentryTheme) Icon(name fyne.ThemeIconName) fyne.Resource { return t.base.Icon(name) } func (t gosentryTheme) Icon(name fyne.ThemeIconName) fyne.Resource { return t.base.Icon(name) }
func (t gosentryTheme) Size(name fyne.ThemeSizeName) float32 { return t.base.Size(name) } func (t gosentryTheme) Size(name fyne.ThemeSizeName) float32 { return t.base.Size(name) }
// themeFor maps a stored Theme choice to a concrete fyne.Theme. Anything other // themeFor maps a stored Theme choice to a concrete fyne.Theme. Only the
// than the explicit GoSentry choice (including the empty/legacy value) keeps // explicit system choice keeps Fyne's built-in theme; everything else
// Fyne's built-in theme. // (including the empty/legacy value) uses the branded GoSentry theme.
func themeFor(choice domain.Theme) fyne.Theme { func themeFor(choice domain.Theme) fyne.Theme {
if choice == domain.ThemeGoSentry { if choice == domain.ThemeSystem {
return newGoSentryTheme() return theme.DefaultTheme()
} }
return theme.DefaultTheme() return newGoSentryTheme()
} }
// applyTheme installs the theme for the given choice on the running app. Fyne // applyTheme installs the theme for the given choice on the running app. Fyne
+16 -16
View File
@@ -54,32 +54,32 @@ func TestGoSentryThemeDelegatesUnbrandedColors(t *testing.T) {
} }
} }
// themeFor maps the stored choice to the right theme: the GoSentry choice yields // themeFor maps the stored choice to the right theme: the GoSentry choice and the
// the branded teal primary; every other value (including the empty legacy value) // empty legacy value yield the branded teal primary; only the explicit system
// yields the default theme, whose primary is not the brand teal. // choice yields Fyne's built-in theme.
func TestThemeForChoice(t *testing.T) { func TestThemeForChoice(t *testing.T) {
gosentry := themeFor(domain.ThemeGoSentry) for _, choice := range []domain.Theme{domain.ThemeGoSentry, ""} {
if got := gosentry.Color(theme.ColorNamePrimary, theme.VariantLight); got != brandTeal { branded := themeFor(choice)
t.Errorf("themeFor(gosentry) primary = %v, want brand teal %v", got, brandTeal) if got := branded.Color(theme.ColorNamePrimary, theme.VariantLight); got != brandTeal {
} t.Errorf("themeFor(%q) primary = %v, want brand teal %v", choice, got, brandTeal)
for _, choice := range []domain.Theme{domain.ThemeDefault, ""} {
def := themeFor(choice)
if got := def.Color(theme.ColorNamePrimary, theme.VariantLight); got == brandTeal {
t.Errorf("themeFor(%q) should not use the brand teal primary", choice)
} }
} }
sys := themeFor(domain.ThemeSystem)
if got := sys.Color(theme.ColorNamePrimary, theme.VariantLight); got == brandTeal {
t.Errorf("themeFor(system) should not use the brand teal primary")
}
} }
// The dropdown label helpers must round-trip, and the empty/legacy value must map // The dropdown label helpers must round-trip, and the empty/legacy value must map
// to the Default label so the select never shows a blank option. // to the GoSentry label so the select never shows a blank option.
func TestThemeLabelRoundTrip(t *testing.T) { func TestThemeLabelRoundTrip(t *testing.T) {
if got := themeFromLabel(themeLabel(domain.ThemeGoSentry)); got != domain.ThemeGoSentry { if got := themeFromLabel(themeLabel(domain.ThemeGoSentry)); got != domain.ThemeGoSentry {
t.Errorf("round-trip gosentry = %q", got) t.Errorf("round-trip gosentry = %q", got)
} }
if got := themeFromLabel(themeLabel(domain.ThemeDefault)); got != domain.ThemeDefault { if got := themeFromLabel(themeLabel(domain.ThemeSystem)); got != domain.ThemeSystem {
t.Errorf("round-trip default = %q", got) t.Errorf("round-trip system = %q", got)
} }
if got := themeLabel(""); got != themeLabelDefault { if got := themeLabel(""); got != themeLabelGoSentry {
t.Errorf("empty theme label = %q, want %q", got, themeLabelDefault) t.Errorf("empty theme label = %q, want %q", got, themeLabelGoSentry)
} }
} }
+51 -22
View File
@@ -4,12 +4,46 @@ import (
"runtime" "runtime"
"gitea.mixdep.ru/mix/gosentry/assets" "gitea.mixdep.ru/mix/gosentry/assets"
"gitea.mixdep.ru/mix/gosentry/src/domain"
"fyne.io/fyne/v2" "fyne.io/fyne/v2"
fynedesktop "fyne.io/fyne/v2/driver/desktop" fynedesktop "fyne.io/fyne/v2/driver/desktop"
) )
func configureSystemTray(a fyne.App, w fyne.Window) { // systemTrayRegistered tracks whether this process registered a tray icon at
// launch. Fyne cannot add or remove the icon mid-session, so toggling
// KeepRunningInTray in Settings updates close behavior immediately and shows a
// restart hint for the icon itself.
var systemTrayRegistered bool
// mainWindowHidden tracks whether the primary window was hidden via the tray
// close intercept. Fyne exposes no Window.Visible API, so the flag drives the
// reveal-on-tray-disable path in applyTrayBehavior.
var mainWindowHidden bool
const trayRestartHintText = "Restart GoSentry for the tray icon change to take effect."
func resolveStartHidden(cliStartInTray, keepInTray bool) bool {
return domain.ResolveStartHidden(cliStartInTray, keepInTray)
}
// applyTrayBehavior configures window close handling for KeepRunningInTray.
// When revealIfHidden is true and the tray is off, a hidden window is shown so
// the user can still reach the app after disabling the tray mid-session.
func applyTrayBehavior(a fyne.App, w fyne.Window, keepInTray bool, revealIfHidden bool) {
if keepInTray && !systemTrayRegistered {
registerSystemTray(a, w)
systemTrayRegistered = true
}
setWindowCloseBehavior(w, keepInTray)
if !keepInTray && revealIfHidden && mainWindowHidden {
mainWindowHidden = false
w.Show()
w.RequestFocus()
}
}
func 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
@@ -34,26 +68,13 @@ func configureSystemTray(a fyne.App, w fyne.Window) {
// localized label — which our literal "Quit" does not. Setting IsQuit makes // localized label — which our literal "Quit" does not. Setting IsQuit makes
// Fyne reuse this item instead of adding a duplicate, regardless of locale. // Fyne reuse this item instead of adding a duplicate, regardless of locale.
// Window size persistence is frozen: w.Canvas().Size() returns the maximized
// dimensions when the window is maximized, so saving here would corrupt the
// stored size. Needs cross-platform maximized-state detection (IsZoomed /
// _NET_WM_STATE / NSWindow.isZoomed) before it can be re-enabled safely.
// See ROADMAP.md — "Window size — skip saving when maximized".
//
// saveWindowSize := func() {
// size := w.Canvas().Size()
// prefs := a.Preferences()
// prefs.SetFloat("window.width", float64(size.Width))
// prefs.SetFloat("window.height", float64(size.Height))
// }
quit := fyne.NewMenuItem("Quit", func() { quit := fyne.NewMenuItem("Quit", func() {
// saveWindowSize()
a.Quit() a.Quit()
}) })
quit.IsQuit = true quit.IsQuit = true
menu := fyne.NewMenu("GoSentry", menu := fyne.NewMenu("GoSentry",
fyne.NewMenuItem("Show", func() { fyne.NewMenuItem("Show", func() {
mainWindowHidden = false
w.Show() w.Show()
w.RequestFocus() w.RequestFocus()
}), }),
@@ -62,11 +83,19 @@ func configureSystemTray(a fyne.App, w fyne.Window) {
) )
desk.SetSystemTrayMenu(menu) desk.SetSystemTrayMenu(menu)
desk.SetSystemTrayWindow(w) desk.SetSystemTrayWindow(w)
w.SetCloseIntercept(func() { }
// Closing hides the window instead of quitting because scheduler tools are
// expected to keep working in the background. The explicit Quit tray item func setWindowCloseBehavior(w fyne.Window, keepInTray bool) {
// remains the way to stop the process. if keepInTray {
// saveWindowSize() w.SetCloseIntercept(func() {
w.Hide() // Closing hides the window instead of quitting because scheduler tools are
}) // expected to keep working in the background. The explicit Quit tray item
// remains the way to stop the process.
mainWindowHidden = true
w.Hide()
})
return
}
mainWindowHidden = false
w.SetCloseIntercept(nil)
} }
+14
View File
@@ -0,0 +1,14 @@
package ui
import "testing"
func TestResolveStartHiddenUsesDomainHelper(t *testing.T) {
// resolveStartHidden is the UI alias used at startup; it must stay aligned
// with domain.ResolveStartHidden so run.go and tests share one definition.
if got := resolveStartHidden(true, false); got {
t.Fatal("expected hidden start to require both CLI flag and keepInTray")
}
if !resolveStartHidden(true, true) {
t.Fatal("expected hidden start when both flags are set")
}
}