gosentry/docs/REFACTORING.md

GoSentry Refactoring Plan

Status: proposed — not yet started. Goal: make the codebase solid, comprehensive, and human-readable / maintainable without changing observable behavior.

This document is the single source of truth for the refactor. It records the target architecture, the rationale, and a sequence of small, independently reviewable tasks. Each task lists the recommended agent model and effort level.

---

1. Why refactor

The application works and is well-commented, but its structure does not scale:

#	Problem	Impact
1	src/gui/app.go is a 1,057-line monolith	Nothing can be found, reused, or tested in isolation
2	src/core is one flat package mixing 7 concerns	No boundaries; everything can call everything
3	Shared mutable *[]Job between GUI and Scheduler	GUI mutates the slice with no lock; scheduler locks the same slice → data race
4	onChange mutates Fyne widgets from the scheduler goroutine	Latent crash/corruption — Fyne requires UI updates on the main thread
5	Job mixes durable config and runtime state (yaml:"-" fields)	The "noise" the model fights to exclude lives in the same struct
6	Errors swallowed everywhere (_ = store.SaveJobs(...))	Save failures are invisible to the user
7	No service/controller layer; GUI reaches into store.Paths, drives scheduler directly	Business logic is tangled into widget callbacks
8	Schedule strings re-parsed every tick; no Schedule value type	Validation scattered; no single source of truth
9	Tests only cover core; GUI and orchestration untestable	Documented gap in docs/TESTS.md

Note on layout: the project intentionally keeps the src/ directory. The src/ → internal/ move was considered and rejected — it is cosmetic for a non-imported desktop app and not worth the import-path churn. All packages below live under src/.

---

2. Target architecture

The central change is to insert an application-service layer that owns all state and exposes intent-based methods. This turns the UI into a thin view and the core packages into stateless engines, dissolving problems 3, 4, 6, and 7.

┌──────────────┐   intents    ┌─────────────────┐   calls    ┌──────────────┐
│   ui (Fyne)  │ ───────────▶ │   app.Service   │ ─────────▶ │ core engines │
│  thin views  │ ◀─────────── │  (sole owner of │            │ scheduler /  │
│ fyne.Do only │   events     │  state + mutex) │ ◀───────── │ runner /     │
└──────────────┘              └─────────────────┘  records   │ storage      │
                                                              └──────────────┘

• One writer. app.Service holds the job list + runtime state behind a mutex. The UI never mutates state directly — it calls CreateJob, RunNow, SetGlobalPause, etc.
• Events flow back through an observer interface. The UI's listener is the only place that touches widgets, and it marshals onto the main thread with fyne.Do.
• Core engines are stateless / injected — scheduler and runner operate on data passed in, not a shared slice.

2.1 Package layout (all under src/)

cmd/gosentry/
  main.go                  # flag parse → ui.Run

src/
  domain/                  # pure types, zero external deps
    job.go                 # Job (durable config only — no yaml:"-")
    runtime.go             # JobRuntime (LastRun/NextRun/State/Output/Logs)
    record.go              # RunRecord
    config.go              # Config + StartInTrayArgument
    schedule.go            # Schedule value object: Parse / Validate / Next()

  storage/                 # persistence + path resolution + migration
    store.go               # Load/SaveConfig, Load/SaveJobs
    paths.go               # ResolvePaths
    yaml.go                # writeYAML helper
    migration.go           # pysentry → gosentry legacy handling

  scheduler/
    scheduler.go           # timing loop; drives Service via callbacks
    clock.go               # Clock interface (real + fake for tests)

  runner/
    runner.go              # RunJob orchestration
    invocation.go          # build exec.Cmd (shared)
    invocation_windows.go  # cmd.exe quoting
    invocation_other.go    # sh -c
    exitcodes.go           # parse / accept success codes
    logfile.go             # writeRunLog + sanitizeFileName
    cleanup.go             # CleanupLogs

  platform/
    winproc/               # hidden-window helper shared by runner + autostart
      winproc_windows.go   # CREATE_NO_WINDOW / HideWindow
      winproc_other.go     # no-op
    autostart/
      autostart.go         # Manager interface + Status type
      windows.go linux.go other.go
    desktop/
      desktop_linux.go other.go

  app/
    service.go             # owns state; CreateJob/UpdateJob/Delete/RunNow/...
    events.go              # Event types + Observer registration
    format.go              # display strings (moved out of GUI)

  ui/                      # renamed from src/gui; thin Fyne views
    run.go                 # Run(): lifecycle, window, tray wiring
    mainwindow.go          # tab assembly + event listener (fyne.Do)
    jobs_view.go           # list + details panel + toolbar
    job_dialog.go          # new/edit form
    history_view.go        # history table
    settings_view.go       # settings form
    tray.go                # system tray
    singleinstance.go      # localhost IPC
    layout.go              # minWidthLayout

Import paths follow the existing convention, e.g. gitea.mixdep.ru/mix/gosentry/src/domain, gitea.mixdep.ru/mix/gosentry/src/app.

2.2 Dependency direction (must stay acyclic)

domain   ← (no deps)
storage  ← domain
runner   ← domain, platform/winproc
scheduler← domain
app      ← domain, storage, scheduler, runner
ui       ← app, domain (Fyne)
platform/autostart, platform/desktop ← (own deps; winproc for windows)
cmd      ← ui

2.3 Key design decisions

1. Split durable vs. runtime in the domain. domain.Job becomes pure YAML config (no yaml:"-"). Runtime state moves to domain.JobRuntime, held by the service keyed by job ID. (Resolves #5.)
2. Schedule value object. schedule.Parse(string) (Schedule, error) validates once and exposes Next(time.Time). (Resolves #8.)
3. Autostart behind a Manager interface, selected per platform — mockable, no package-level functions.
4. Injectable Clock in the scheduler → deterministic tests.
5. Errors surface to the UI. Service methods return errors; status bar shows them. No more _ = on saves. (Resolves #6.)
6. Thread-safety contract: core engines never import Fyne; the UI listener is the sole widget mutator and always wraps updates in fyne.Do. (Resolves #4.)

---

3. Task sequence

Tasks are ordered so the tree compiles and all tests pass after every task. Each task is a small, reviewable unit.

Model guidance

• haiku — mechanical moves, renames, no judgment required.
• sonnet — localized logic changes with clear scope.
• opus — architecture-shaping work (new layers, concurrency, public APIs).

Effort guidance — reasoning depth, not size: low / medium / high.

Phase 0 — Safety net

Task	Description	Model	Effort
T0.1	Add scripts/test.sh + .bat running go vet ./... and go test -race ./.... Document in docs/TESTS.md.	haiku	low
T0.2	Add characterization tests that pin current behavior at seams to be moved: store load→save round-trip, scheduler nextRunTime, end-to-end RunJob log output. (Some exist; fill gaps.)	sonnet	medium

Phase 1 — Split the flat core package (no logic change)

Mechanical moves + import fixes only. Behavior identical.

Task	Description	Model	Effort
T1.1	Create src/domain; move Job, RunRecord, Config, JobsFile, StartInTrayArgument from model.go. Keep yaml:"-" fields for now (split happens in Phase 2). Update all references.	sonnet	medium
T1.2	Create src/platform/winproc; move configureHiddenWindow + hidden-window flags out of runner_windows.go / runner_other.go. This breaks the future autostart→runner coupling early.	sonnet	medium
T1.3	Create src/runner; move runner.go, runner_windows.go, runner_other.go, runner_test.go. Point at winproc. Split helpers into invocation*.go, exitcodes.go, logfile.go, cleanup.go as the file moves.	sonnet	medium
T1.4	Create src/scheduler; move scheduler.go, scheduler_test.go. Still takes *[]domain.Job for now.	sonnet	medium
T1.5	Create src/storage; move store.go, paths.go, store_test.go.	sonnet	medium
T1.6	Create src/platform/autostart; move autostart_*.go + tests. Point at winproc.	sonnet	medium
T1.7	Create src/platform/desktop; move desktop_linux.go, desktop_other.go.	haiku	low
T1.8	Delete the now-empty src/core; run full build + tests on both platforms (or with build tags) to confirm parity.	haiku	low

Phase 2 — Domain cleanup

Task	Description	Model	Effort
T2.1	Add src/domain/schedule.go: Schedule value object with Parse, Validate, Next(time.Time). Unit-test it. Keep nextRunTime as a thin wrapper initially.	opus	high
T2.2	Migrate scheduler to use Schedule (parse on load/edit, not per tick). Remove duplicated parsing.	sonnet	medium
T2.3	Split domain.Job (durable) from domain.JobRuntime (transient). Remove all yaml:"-" fields and nextDue from Job. Add runtime.go.	opus	high
T2.4	Update storage: load/save only Job; move runtime initialization out of normalizeJobs into a domain.NewRuntime(job) constructor. Update round-trip tests.	sonnet	medium

After Phase 2 the scheduler and GUI still share state; the Job/JobRuntime split is wired through temporary glue. Phase 3 removes the sharing.

Phase 3 — Application service layer

Task	Description	Model	Effort
T3.1	Create src/app/service.go: Service owning []domain.Job + map[int]*domain.JobRuntime behind a sync.Mutex. Constructor wires storage.	opus	high
T3.2	Add src/app/events.go: Event types (job changed, run recorded, scheduler state) + Observer registration. Single-threaded dispatch contract documented.	opus	high
T3.3	Move state-mutating operations into the service: CreateJob, UpdateJob, DeleteJob, SetEnabled, RunNow, SetGlobalPause, UpdateSettings. Each returns error.	opus	high
T3.4	Convert scheduler to operate through the service (no *[]Job). Scheduler asks the service for due jobs and reports records back; service is the sole writer. Inject Clock.	opus	high
T3.5	Move display/format helpers (displayFolder, displayArguments, displayRunMode, statusText, …) from GUI into src/app/format.go.	haiku	low
T3.6	Add src/app unit tests (no Fyne): create/edit/delete, enable/pause, global pause, run-now path with a fake runner + fake clock. Big coverage win.	opus	high

Phase 4 — Carve up the GUI

Rename src/gui → src/ui and break app.go into focused files. The UI now talks only to app.Service and reacts to events via fyne.Do.

Task	Description	Model	Effort
T4.1	Rename package gui → ui; split lifecycle into run.go + mainwindow.go. Wire the event listener and route every widget update through fyne.Do. (Resolves #4.)	opus	high
T4.2	Extract jobs_view.go (list + details + toolbar), driven by service calls + events.	sonnet	medium
T4.3	Extract job_dialog.go; validate schedule via domain.Schedule.Validate.	sonnet	medium
T4.4	Extract history_view.go.	sonnet	medium
T4.5	Extract settings_view.go; surface save/autostart/cleanup errors to the status label. (Resolves #6 in UI.)	sonnet	medium
T4.6	Extract tray.go, singleinstance.go, layout.go.	haiku	low
T4.7	Confirm app.go is gone and ui imports only app + domain + Fyne. Manual smoke test on each platform.	sonnet	medium

Phase 5 — Hardening & docs

Task	Description	Model	Effort
T5.1	Replace remaining _ = ...Save... with propagated/surfaced errors across service + storage.	sonnet	medium
T5.2	Introduce autostart.Manager interface + per-platform impls; inject into the service instead of calling package funcs.	sonnet	medium
T5.3	Fill documented test gaps: folder filtering, log cleanup (count + age), settings persistence/migration, concurrent run prevention.	sonnet	high
T5.4	Run go test -race ./... clean. Confirm no data race remains.	haiku	low
T5.5	Update docs/ARCHITECTURE.md, docs/TESTS.md, and the README "Project Layout" section to the new structure.	sonnet	medium

---

3.1 Task completion checklist

Track progress here. Mark tasks complete as they land and pass review.

Phase 0 — Safety net

• T0.1 — Add test script + go vet + go test -race
• T0.2 — Add characterization tests

Phase 1 — Split flat core package

• T1.1 — Create src/domain; move Job/RunRecord/Config/etc
• T1.2 — Create src/platform/winproc; move configureHiddenWindow
• T1.3 — Create src/runner; move runner logic
• T1.4 — Create src/scheduler; move scheduler
• T1.5 — Create src/storage; move store/paths
• T1.6 — Create src/platform/autostart; move autostart logic
• T1.7 — Create src/platform/desktop; move desktop integration
• T1.8 — Delete empty src/core; build + test both platforms

Phase 2 — Domain cleanup

• T2.1 — Add src/domain/schedule.go; Schedule value object
• T2.2 — Migrate scheduler to use Schedule
• T2.3 — Split domain.Job (durable) from domain.JobRuntime (transient)
• T2.4 — Update storage: load/save Job only; move runtime init

Phase 3 — Application service layer

• T3.1 — Create src/app/service.go; owns state behind mutex
• T3.2 — Add src/app/events.go; Event types + Observer
• T3.3 — Add state-mutating operations to service
• T3.4 — Convert scheduler to use service; inject Clock
• T3.5 — Move display helpers to src/app/format.go
• T3.6 — Add src/app unit tests (no Fyne)

Phase 4 — Carve up the GUI

• T4.1 — Rename gui → ui; split app.go into run.go + mainwindow.go
• T4.2 — Extract jobs_view.go
• T4.3 — Extract job_dialog.go
• T4.4 — Extract history_view.go
• T4.5 — Extract settings_view.go
• T4.6 — Extract tray.go, singleinstance.go, layout.go
• T4.7 — Confirm app.go is gone; smoke test both platforms

Phase 5 — Hardening & docs

• T5.1 — Surface errors from service + storage
• T5.2 — Introduce autostart.Manager interface
• T5.3 — Fill test gaps (folder filtering, cleanup, migration, concurrency)
• T5.4 — Run go test -race ./... clean on both platforms
• T5.5 — Update docs (ARCHITECTURE.md, TESTS.md, README)

---

4. Definition of done

• go vet ./... clean; go test -race ./... green on Windows and Linux.
• No package outside ui imports Fyne; no engine mutates UI state.
• domain.Job has no yaml:"-" fields.
• app.Service is the only writer of job/runtime state.
• src/ui contains no file over ~250 lines; no single file over ~400.
• docs/ARCHITECTURE.md matches the shipped structure.

5. Risks & mitigations

Risk	Mitigation
Cross-platform code moves break the non-host OS build	Build with both GOOS=windows and GOOS=linux after each platform-touching task (T1.2, T1.3, T1.6, T1.7).
Concurrency change (Phase 3/4) introduces subtle deadlocks	Keep the service mutex non-reentrant; never call back into the UI while holding it; cover with -race tests in T3.6.
Behavior drift during moves	Characterization tests (T0.2) pin behavior before structural change.
Large diff hard to review	Each task is a separate commit/PR; phases land independently.