Phase 11 of PROJECT_REVIEW_PLAN.md: the themed cleanup pass over every
low-severity finding still open (2.2-2.3, 3.4-3.6, 4.3-4.7, 6.4-6.7,
7.1-7.3, 8.2-8.3, 9.1-9.4, and the under-documented decisions in §10/§11).
Behavioral fixes:
- Reassign duplicate job IDs in a hand-edited jobs.json instead of letting
two jobs share one runtime, schedule entry, and SeedStats bucket.
- Disambiguate run-log file names that collide within the same second.
- Compute AvgDurationMS as DurationSumMS/TimedRunCount instead of an
incremental integer mean, so it always matches the seeded-from-logs
average instead of drifting from truncation error.
- Clean absolute paths in ResolveConfiguredPath so two spellings of the
same jobs file do not trigger a spurious adoption.
- Report InstallDesktopIcon failures through ErrorOccurred instead of
discarding them silently.
- Move settingsView's blocking AutostartStatus (PowerShell on Windows) off
the UI thread.
- Give notify-timing.tsv its own extension so CleanupLogs no longer
manages it as a run log.
- Replace the settingsView Save handler's second copy of validateConfig's
rules with a bare parse, letting the Service's own error surface.
Cleanups:
- Delete collectActivity, the dead yaml tags on RunRecord, and the
logArguments/LogArguments alias.
- Fold the two systemTrayRegistered/mainWindowHidden globals into one
trayState instance Run owns and threads through Settings and the
single-instance reveal path.
- Fix stale comments/docs: the frozen window-size restore claim, a
reference to a renamed recordRun, README's "Pause all" and notification
wording, the PowerShell quoting note for TESTS.md's coverage command,
and scripts/test.bat's UTF-8 checkmarks under a non-UTF-8 code page.
- Document the single-instance fallback's consequence and the
unauthenticated instance-channel port in STANDARDS.md; record the
config-shim retirement plan in ROADMAP.md.
3.5, 7.3, and 9.4 turned out to already be fixed by earlier phases; no
change needed for those three.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 10 of the whole-project review (findings 5.1 and 5.2), folded into
the ROADMAP file-split item as that plan asks.
5.2 was a real defect. `selected` was an index into a snapshot of the jobs
slice, and every path that changed the slice patched it by hand. The one
path that could not — adopting a different jobs file, where the Service
replaces the whole list and the view only hears about it through the
refresh JobsLoaded triggers — left the details pane redrawing from an
index that belonged to the previous list, describing whichever job now sat
there (or clearing when the new list was shorter) while the list highlight
stayed put. The selection is now a job ID; rows are derived from it at
render time, and refresh ends by pointing the highlight at the selected
job, so the two can no longer disagree.
5.1: newJobsView was one 330-line constructor whose dozen closures shared
seven mutable locals. It is now a jobsView struct over a jobsViewState
that owns the snapshot, the folder filter, and the selection — the
invariant that used to be maintained by hand in five places lives in one
place — split across jobs_view.go (construction, refresh, layout),
jobs_view_state.go, jobs_view_list.go, and jobs_view_toolbar.go. The
folder-option rebuild that appeared verbatim in three handlers is one
method.
Behaviour that changed beyond the fix: switching the folder filter keeps
the current selection when the new filter still shows it, instead of
always jumping to the folder's first job.
Docs: ARCHITECTURE records the new file layout and the selection-by-ID
contract; ROADMAP drops jobs_view.go from the over-guideline table and
refreshes the other five numbers (finding 2.4); TESTS documents the new
state test file and the adoption regression test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 7 of the whole-project review (findings 3.2 and 3.3).
Service.mu is the lock the Fyne main thread takes on every Jobs() and
Runtime() call, so anything blocking inside it makes a UI refresh wait on
the disk. Three things did:
- Every SaveJobs/SaveConfig was a marshal, fsync, and rename under mu.
Writes are now prepared under the lock (Store.PrepareSaveJobs /
PrepareSaveConfig snapshot the payload and target path) and run after
it is released. deferSaveLocked takes saveMu while mu is still held, so
writes still reach the file in the order their snapshots were taken and
an older snapshot can never land on top of a newer one.
- executeRun ran runner.CleanupLogs under mu after every run. It needs
only the values already snapshotted into runEnv, so it now runs after
the unlock — including when the job is gone, since the run still wrote
a log file that retention covers.
- adoptJobsLocked ran runner.SeedStats under mu, reached from
UpdateSettings on the UI thread. Seeding moved out into
applySeededStatsLocked; UpdateSettings now reads the new jobs file and
seeds its statistics before taking the lock, and re-checks the
"no jobs-file switch while running" guard once it has it.
SeedStats also opened every log file twice — once to find the job, again
to read the result. readLogSummary reads job_id, state, and duration in
one pass, so each log is opened once.
StartOnly runs were built with exec.CommandContext on the app's lifecycle
context. os/exec keeps a watcher goroutine alive until Wait returns or the
context is done, and StartOnly never calls Wait, so one goroutine leaked
per run and would then try to kill a process whose handle startJobOnly had
already released. The invocation now uses context.Background(), whose nil
Done channel means no watcher is started at all.
Regression tests: TestRunJobStartOnlyLeavesNoContextWatcher (fails with 5
leaked goroutines on the old code), TestConcurrentJobOperationsLeaveTheFileMatchingMemory,
and TestUpdateSettingsSeedsAdoptedJobsFromLogs. STANDARDS gains the
no-I/O-under-mu rule and the "a StartOnly process outlives GoSentry" entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
History was appended to on every recorded run and never trimmed, and every
event re-sorted the whole slice and re-measured the Job, Detail and Log
columns across every row. The per-run cost therefore grew with the number of
rows, in exactly the mode the app is designed for: left in the tray for days.
The session History now keeps the newest maxHistoryRows (1000) records, the
way maxJobLogs caps a job's own activity list, and drops the oldest from the
front, zeroing the tail so a dropped record's full captured output is not
kept alive by the backing array. Column widths move into a historyLog value
that folds each new record into the current maxima instead of rescanning.
Widths only grow within a theme, so a column never narrows when a record ages
out; a theme change is the one case that still rescans, because every stored
width was measured at the old text size.
Measured with a throwaway benchmark over 5000 accumulated records: one
refresh went from 15.8 ms to 0.9 ms. At the new cap the full width rescan
alone costs 1.5 ms, so both halves of the fix carry weight.
Plan item 6 of docs/PROJECT_REVIEW_PLAN.md (finding 3.1).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implements items 4-5 of the whole-project review's suggested order
(docs/PROJECT_REVIEW_PLAN.md):
- Drop the three SaveJobs calls in the run lifecycle (startRunLocked,
executeRun, SetGlobalPause): none of them change a durable Job field,
everything they touch lives on JobRuntime, which is never persisted.
Retire TestStartRunLockedRollbackOnSaveFailure with the rollback it
guarded, since a run can no longer fail to start this way.
- Clear PendingRuns (the "queue" overlap policy's backlog) when a job is
disabled or the scheduler is globally paused, so resuming or
re-enabling a job no longer replays a deferred run left over from
before the pause/disable. Cap it at maxPendingRuns (10) so a job whose
runs take longer than its own interval stops accumulating an unbounded
backlog. Surface the queued count in the details pane via DisplayStats.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
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>
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>
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>
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>
The document had drifted well past the point of being useful as an index:
it listed 130 tests against 170 in the tree, described four test files that
were never added to it at all (domain/config_test.go, runner/seed_test.go,
ui/layout_test.go, ui/theme_test.go), attributed runner's Windows
invocation tests to the wrong file, kept two tests that no longer exist
(TestParseRegistryRunValue, TestLinuxAutostartRemovesLegacyDesktopEntry),
and filed three activity-panel tests under jobs_view_test.go when they live
in history_view_test.go.
Most of the gap is the work of the last few releases: the per-job timeout's
three states, the persisted pause and jobs-list density, SeedStats, and the
whole GUI-geometry set the layout review produced.
Three claims were false rather than merely missing. The ui test files are
no longer "pure helpers, no Fyne widget construction" — they build views
under test.NewApp() and measure them, which is now stated as its own design
principle, because that measurement is what makes the STANDARDS rule about
theme-derived sizes enforceable. layout.go is no longer an open coverage
gap. And the coverage-gap list now names the real remaining one: Fyne's
headless driver cannot report a maximized window, which is the reason
window-size persistence is frozen.
Verified mechanically: every test function in the tree appears exactly once
in the document, under the file it actually lives in, and the document
names no test that does not exist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Jobs directory row named a folder and assumed the file inside it was
called jobs.json. It is now a Jobs file row: Browse opens a file picker
filtered to .json, the field stays editable so a file that does not exist yet
can be typed, and the job list can live under any name.
Config.JobsDir/jobs_dir becomes Config.JobsFile/jobs_file, holding the whole
path; Paths.JobsDir is derived from it so saves still create the folder. An
older gosentry.json is migrated on load by joining its jobs_dir with
jobs.json — the exact file that version used — and the retired key is dropped
when the config is rewritten. The default clears before unmarshalling, or a
file that omits jobs_file and a file that sets it would be indistinguishable
and the migration would never run.
Saving used to write the current job list over whatever was at the new path,
which made switching to an existing jobs file impossible: its contents were
destroyed. An existing file now wins. Its jobs are loaded, normalized, and
adopted, with runtimes, schedule cache, next-run times and log-seeded
statistics rebuilt around them by adoptJobsLocked — the same helper NewService
now uses, so construction and adoption cannot drift. A path with no file
behind it still receives the current jobs, which is how the file is renamed or
relocated. The new file is read before anything is written, so an unparsable
one leaves both the config and the jobs untouched.
Adoption drops every runtime, and a run finishing afterwards would write its
result onto whichever job inherited its ID, so the switch is refused while a
job is running. Unrelated settings still save during a run. Because the
replacement happens without a prompt, the Service emits JobsLoaded with the
path and count, and History carries the receipt.
A path that names only a folder (trailing separator, a dot, or two dots) is
rejected with a validation error instead of failing later with an opaque OS
error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reading a log file meant copying the configured path out of Settings and
pasting it into a file manager. The Logs directory row now carries an Open
button beside Browse that reveals the folder directly.
The new src/platform/filemanager package holds the platform split — explorer
on Windows, xdg-open on Linux, an "unsupported" error elsewhere — and starts
the handler without waiting on it, since Explorer exits non-zero even after it
opens the window and blocking would stall the UI thread. A missing path, a
path that is a file, and a handler that will not start are all reported to the
user; the logs directory does not exist until the first run, so that case is
reachable.
The button opens whatever the field currently holds rather than the saved
config, so an edit can be checked before Save. Resolving a relative directory
against the application folder is the store's rule, so resolveConfiguredDir is
now exported as storage.ResolveConfiguredDir instead of being duplicated in
the UI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replace CODE_REVIEW.md with a living maturity checklist, document
session-only History, inject Service into newMainView for testability,
add UI and scheduler regression tests, and fix RunNow error surfacing
plus empty jobs view handling.
Co-authored-by: Cursor <cursoragent@cursor.com>
Snapshot store paths under lock before async runs, roll back failed
start/save state, emit UI events only after successful persistence,
surface log write failures, and sync stale YAML docs to JSON.
Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary
Completed Phase 5 refactoring and reached the target architecture.
**Architectural milestone achieved:**
- Service layer owns all state and is the sole writer
- UI is a thin Fyne view, all widget updates marshaled via `fyne.Do`
- Core engines are stateless and injectable
- Domain types are pure (no `yaml:"-"` fields)
- Full module builds and `go vet ./...` clean
## Changes
- Bump version: 0.3.6 → 0.4.0
- Update CHANGELOG with Phase 5 summary
- Add ROADMAP "Refactoring Follow-Ups" section
## Known follow-up work
1. **Linux test build broken** — `runner_test.go` needs `//go:build windows` tag
2. **File-size limits exceeded** — `operations.go` (486 lines), `jobs_view.go` (415 lines)
See ROADMAP.md for details.
---------
Co-authored-by: mixeme <mix.public@ya.ru>
Reviewed-on: #1
Created docs/TESTS.md documenting all 25 tests across 5 test files:
- store_test.go: YAML serialization tests
- scheduler_test.go: Schedule parsing and invocation output tests
- runner_test.go: Command execution, exit codes, and Windows process tests
- autostart_windows_test.go: Windows startup folder shortcut creation tests
- autostart_linux_test.go: Linux XDG Desktop Entry autostart tests
Includes test descriptions, platform requirements, and usage instructions.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
mix
mixeme