diff --git a/.gitignore b/.gitignore index 531debc..1b1cedc 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ # built binary /sysmon -sysmon # macOS .DS_Store diff --git a/.golangci.yml b/.golangci.yml index b6c039b..0557971 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,6 +20,8 @@ formatters: goimports: local-prefixes: - sysmon + gofumpt: + module-path: sysmon linters: enable: diff --git a/.planning/PROJECT.md b/.planning/PROJECT.md index 1e30d98..cbcd42c 100644 --- a/.planning/PROJECT.md +++ b/.planning/PROJECT.md @@ -55,7 +55,7 @@ nearly free in resource use. ## Constraints - **Tech stack**: Go 1.26 + cgo, macOS only (arm64) — uses CoreGraphics/ApplicationServices C API. -- **Dependencies**: standard library only — new third-party deps require justification. +- **Dependencies**: runtime/production is standard-library only (zero third-party in the binary — reason `log/slog` was chosen). Test-only deps are allowed: `testify` (assertions), `go.uber.org/mock`/mockgen (mocks from interfaces), `testcontainers` (integration tests only). - **Platform**: requires Xcode Command Line Tools and macOS Accessibility permission. - **Resource use**: must stay lightweight — near-zero idle CPU, tiny memory footprint, battery-friendly (runs for hours in background). @@ -71,6 +71,7 @@ nearly free in resource use. | Structured logging via stdlib log/slog | Levels/structure without breaking the zero-dep constraint (vs zerolog/zap) | — Pending | | CI on macos-latest (build/lint/test) | cgo + CoreGraphics only compiles on macOS | ✓ Good | | golangci-lint v2 (strict golden config, trimmed) | Consistent strict linting, minus project-specific/web linters | ✓ Good | +| Test stack: testify + mockgen (from interfaces) + testcontainers | Standard Go test tooling; mocks generated, not hand-written (test-only deps, binary stays zero-dep) | — Pending | ## Evolution diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 8975e70..5b16d6b 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -18,10 +18,11 @@ Milestone: v1 — Hexagonal Refactor 3. `KeeperService` (in `service`) depends only on `domain` and `port` types, never imports `adapter`, and runs the tick loop via the `Keeper`/`Pointer` interfaces 4. `CGPointer` adapter implements `Pointer` using cgo/CoreGraphics and is isolated behind a `//go:build darwin` tag 5. `go build` succeeds, all Makefile targets (build/run/start/stop/status/clean) work unchanged, and `go test ./...` passes a `KeeperService` unit test using a fake `Pointer` that verifies nudges fire on the interval without moving the real mouse and without requiring macOS Accessibility permission -**Plans**: TBD +**Plans**: 1 plan +- [ ] 01-01-PLAN.md — Restructure flat main.go into cmd/sysmon + internal/activity {domain,port,service,adapter}, add mockgen+testify+synctest unit test, inject slog logging, update Makefile, remove root main.go; behavior preserved ## Progress | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| -| 1. Hexagonal Refactor | 0/? | Not started | - | +| 1. Hexagonal Refactor | 0/1 | Not started | - | diff --git a/.planning/config.json b/.planning/config.json index 017f9c0..5c2b0f7 100644 --- a/.planning/config.json +++ b/.planning/config.json @@ -45,7 +45,8 @@ "post_planning_gaps": true, "security_enforcement": true, "security_asvs_level": 1, - "security_block_on": "high" + "security_block_on": "high", + "_auto_chain_active": false }, "ship": { "pr_body_sections": [] diff --git a/.planning/phases/01-hexagonal-refactor/01-01-PLAN.md b/.planning/phases/01-hexagonal-refactor/01-01-PLAN.md new file mode 100644 index 0000000..9835534 --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-01-PLAN.md @@ -0,0 +1,391 @@ +--- +phase: 01-hexagonal-refactor +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - go.mod + - go.sum + - main.go + - Makefile + - cmd/sysmon/main.go + - internal/activity/domain/activity.go + - internal/activity/domain/activity_test.go + - internal/activity/port/pointer.go + - internal/activity/port/keeper.go + - internal/activity/port/mock/mock_pointer.go + - internal/activity/service/keeper_service.go + - internal/activity/service/keeper_service_test.go + - internal/activity/adapter/cgpointer.go +autonomous: false +requirements: + - ARCH-01 + - ARCH-02 + - ARCH-03 + - ARCH-04 + - ARCH-05 + - TEST-01 + - BUILD-01 + - BUILD-02 + - LOG-01 + +must_haves: + truths: + - "`make build` produces a working sysmon binary from ./cmd/sysmon" + - "Running the binary nudges the real cursor +1px and back every interval (default 10s), posting a real kCGEventMouseMoved event that resets the macOS idle timer" + - "SIGINT/SIGTERM shuts the process down cleanly with exit code 0" + - "`-quiet` suppresses the Info lifecycle logs by raising the slog level to warn (output stream unchanged)" + - "`go test -race ./...` passes, including a KeeperService test that asserts nudges-per-tick and stop-on-cancel using a mockgen mock, testify, and testing/synctest — moving no real mouse and needing no Accessibility" + - "domain/port/service compile with GOOS=linux (no cgo); only adapter + full binary require darwin" + artifacts: + - cmd/sysmon/main.go + - internal/activity/domain/activity.go + - internal/activity/port/pointer.go + - internal/activity/port/keeper.go + - internal/activity/port/mock/mock_pointer.go + - internal/activity/service/keeper_service.go + - internal/activity/service/keeper_service_test.go + - internal/activity/adapter/cgpointer.go + - Makefile + - go.mod + - go.sum + key_links: + - "cmd/sysmon/main.go constructs adapter.CGPointer and injects it (as a port.Pointer) plus a *slog.Logger into service.NewKeeperService" + - "service.KeeperService.Run drives a time.Ticker and calls port.Pointer.Nudge — never importing adapter" + - "//go:generate directive in port drives mockgen to produce internal/activity/port/mock/mock_pointer.go from the Pointer interface" + - "Makefile build target points at ./cmd/sysmon (not .)" +--- + + +Restructure the flat root `main.go` into a hexagonal ports-&-adapters layout +(`cmd/sysmon` + `internal/activity/{domain,port,service,adapter}`), add a +mockgen+testify+synctest unit test for the ticker loop, and wire in structured +`log/slog` logging — WITHOUT changing any observable runtime behavior. + +Purpose: Make the activity-keeping logic unit-testable and isolate the +macOS/cgo code behind an adapter, following the architecture spec in +docs/TASK.md §10, while preserving the exact current behavior (flags, 1px +nudge-and-back with ~40ms pause, real kCGEventMouseMoved event, 10s default, +clean SIGINT/SIGTERM exit 0, ticker sleep with no busy-wait). + +Output: The four-package hexagon under internal/activity, a new +cmd/sysmon/main.go entry point, a committed generated Pointer mock, a passing +race-enabled unit test, an updated Makefile, and the removed root main.go. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/01-hexagonal-refactor/01-CONTEXT.md +@.planning/phases/01-hexagonal-refactor/01-RESEARCH.md +@.planning/phases/01-hexagonal-refactor/01-VALIDATION.md + +# The behavior-of-record to preserve (cgo nudge block, flags, signals, loop): +@main.go +# Build target path that must change: +@Makefile +# Strict lint config the code must pass (generated mock is exempt via exclusions.generated: strict): +@.golangci.yml +# Authoritative architecture spec: +@docs/TASK.md + + + + + + Task 1: Create the pure domain package (ARCH-04) + internal/activity/domain/activity.go, internal/activity/domain/activity_test.go + + - NewInterval with a positive time.Duration returns an Interval whose Duration() equals the input and a nil error. + - NewInterval with zero or a negative duration returns a non-nil error and the zero Interval. + - (If Offset is modeled as a domain type) NewOffset with a positive int returns that Offset and nil error; NewOffset with zero/negative returns a non-nil error. + + + Create package `domain` implementing ARCH-04 (per D — CONTEXT.md "Structure"). + Add an `Interval` value type wrapping a `time.Duration` with a constructor + `NewInterval(d time.Duration) (Interval, error)` that rejects `d <= 0` + (validation rule from docs/TASK.md §10.3), plus a `Duration()` accessor. + Claude's discretion (CONTEXT.md): model `Offset` either as a validated + domain type (`NewOffset` rejecting `<= 0`) or leave the 1px shift as a plain + constant passed through from cmd — pick the cleaner idiomatic option and keep + it small; do not over-engineer. This package MUST import nothing from + `port`/`service`/`adapter` and MUST NOT import `C`/cgo or `os` (dependency + direction points inward — docs/TASK.md §10.2). `time` and `errors` are the + only stdlib imports needed. Add doc comments on exported identifiers (godot + and revive are enabled in .golangci.yml — end doc comments with a period). + Also add the table-driven `activity_test.go` covering the behavior block + above using testify assertions (`assert`/`require`). + + + cd /Users/katkat/GolandProjects/sysmon && go test ./internal/activity/domain/... && GOOS=linux GOARCH=amd64 go build ./internal/activity/domain/... + + domain package builds on GOOS=linux with no cgo/os imports; NewInterval rejects non-positive durations; domain tests pass. + + + + Task 2: Define the driven and driving ports (ARCH-02, ARCH-03) with a mockgen //go:generate directive + internal/activity/port/pointer.go, internal/activity/port/keeper.go + + Create package `port` with two interfaces (per docs/TASK.md §10.3 and the + locked ports in CONTEXT.md). In `pointer.go`: the driven port + `Pointer` with the single method `Nudge(dx, dy int) error`. In `keeper.go`: + the driving port `Keeper` with `Run(ctx context.Context) error` (import + `context`). Ports know nothing about concrete implementations — no cgo, no + adapter import. Add a committed `//go:generate` directive (on its own line in + `pointer.go`, above the interface) that runs mockgen to generate the Pointer + mock into the `mock` subpackage — this satisfies the LOCKED test-stack + decision to use a mockgen-generated mock, NOT a hand-written fake (CONTEXT.md + Testing, overriding the RESEARCH.md hand-written-fake suggestion). Use the + reflect-mode form targeting this package's `Pointer` interface and writing to + `mock/mock_pointer.go` with package name `mock` (mockgen flags: destination + file under `mock/`, `-package mock`, source or reflect mode selecting + `sysmon/internal/activity/port` and `Pointer`). Add doc comments on the + exported interfaces (godot/revive). + + + cd /Users/katkat/GolandProjects/sysmon && go build ./internal/activity/port/... && grep -q 'go:generate' internal/activity/port/pointer.go && grep -Eq 'Nudge\(dx, dy int\) error' internal/activity/port/pointer.go + + port package builds; Pointer.Nudge(dx, dy int) error and Keeper.Run(ctx) error are defined; a //go:generate mockgen directive is committed in pointer.go. + + + + Task 3: Add test-only deps (testify + go.uber.org/mock) and generate the Pointer mock (TEST-01) + go.mod, go.sum, internal/activity/port/mock/mock_pointer.go + + Add the LOCKED test-only dependencies to go.mod (CONTEXT.md dependency + policy: runtime binary stays stdlib-only; test-only deps are allowed and + expected). Run `go get github.com/stretchr/testify` and + `go get go.uber.org/mock`. Install the mockgen tool + (`go install go.uber.org/mock/mockgen@latest`) and generate the mock by + running `go generate ./...` from the repo root, which executes the directive + added in Task 2 and writes `internal/activity/port/mock/mock_pointer.go`. + The generated file carries the standard "Code generated by MockGen. DO NOT + EDIT." header, so it is exempt from lint under .golangci.yml + `exclusions.generated: strict` — do NOT hand-edit it. Then run `go mod tidy` + so go.mod/go.sum are consistent (testify + go.uber.org/mock and their + transitive deps like davecgh/go-spew, pmezard/go-difflib, gopkg.in/yaml, and + golang.org/x/mod/tools for mockgen appear as indirect/test requires only, + never linked into the production binary). Ensure mockgen is on PATH (it + installs to `$(go env GOPATH)/bin`); if `go generate` cannot find mockgen, + invoke it via the full GOPATH/bin path. + + + cd /Users/katkat/GolandProjects/sysmon && test -f internal/activity/port/mock/mock_pointer.go && head -3 internal/activity/port/mock/mock_pointer.go | grep -q 'DO NOT EDIT' && go build ./internal/activity/port/... && grep -q 'stretchr/testify' go.mod && grep -q 'go.uber.org/mock' go.mod + + go.mod requires testify and go.uber.org/mock (test-only); mock/mock_pointer.go exists with the generated DO-NOT-EDIT header and builds; go.sum is consistent. + + + + Task 4: Implement KeeperService with injected slog logger (ARCH-03, LOG-01) and its synctest unit test (TEST-01) + internal/activity/service/keeper_service.go, internal/activity/service/keeper_service_test.go + + - Given interval I and 3 elapsed ticks (advanced via testing/synctest fake clock), the mock Pointer records exactly 3 nudge pairs: Nudge(+1,0) then Nudge(-1,0) per tick (6 calls total, correct order/args). + - When the context passed to Run is cancelled, Run returns nil and the goroutine exits (no bubble deadlock) — asserting stop-on-context-cancel. + - The 40ms pause between the forward and backward nudge is honored (advanced by the synctest fake clock, not real time). + - Under an injected slog.DiscardHandler logger the test produces no stdout; the test moves no real mouse and requires no Accessibility. + + + Create package `service` with `KeeperService` implementing `port.Keeper` + (ARCH-03). Constructor `NewKeeperService(pointer port.Pointer, logger + *slog.Logger, interval time.Duration, offset int, pause time.Duration)` + (constructor injection per RESEARCH.md Pattern 1 and CONTEXT.md — logger is + injected, never a package global; this is what makes LOG-01 testable). + `Run(ctx context.Context) error` starts a `time.NewTicker(interval)` (defer + Stop), logs an Info "started" line with the interval as a structured attr, + then loops on `select`: on `<-ticker.C` call `Nudge(+offset,0)` → `time.Sleep(pause)` + → `Nudge(-offset,0)`; on `<-ctx.Done()` log Info "stopped" and return nil. + This preserves the exact current loop semantics from main.go (1px there-and-back, + ~40ms pause, ticker sleep — no busy-wait) but behind the port. Depend ONLY on + `domain` and `port` — never import `adapter` (ARCH-03). Use `context`, + `log/slog`, `time`, and `sysmon/internal/activity/port`. + + Also create `keeper_service_test.go` (package `service_test`) covering the + behavior block using the LOCKED test stack: `testing/synctest` (stdlib, + go1.25+) for a deterministic fake clock via `synctest.Test(t, func(t){...})`; + the mockgen-generated mock from `sysmon/internal/activity/port/mock` as the + Pointer (set up a `gomock.NewController(t)`, program `EXPECT().Nudge(...)` + call expectations/counts); and `testify` for assertions. Inject + `slog.New(slog.DiscardHandler)` so the test is silent. Follow RESEARCH.md + Pattern 2 and Pitfall 3: spawn `Run` in a goroutine, advance the fake clock + with `time.Sleep`, call `synctest.Wait()`, assert the recorded nudge pairs, + then ALWAYS `cancel()` + drain the done channel before the bubble closure + returns to avoid a synctest deadlock. Do NOT touch the darwin adapter from + the test (RESEARCH.md Pitfall/anti-patterns — no real mouse, no macOS). + Keep gomock's call-count expectations aligned with synctest's tick count. + + + cd /Users/katkat/GolandProjects/sysmon && go test -race ./internal/activity/service/... && GOOS=linux GOARCH=amd64 go build ./internal/activity/service/... && go list -deps sysmon/internal/activity/service | grep -vq 'internal/activity/adapter' + + KeeperService implements Keeper with an injected *slog.Logger; go test -race for service passes both nudge-per-tick and stop-on-cancel; service builds on GOOS=linux and its dependency list never includes adapter. + + + + Task 5: Relocate the cgo CoreGraphics adapter (ARCH-02) preserving both CFRelease calls verbatim + internal/activity/adapter/cgpointer.go + + Create package `adapter` with `CGPointer` implementing `port.Pointer` via + cgo/CoreGraphics (ARCH-02). The FIRST line of the file MUST be + `//go:build darwin` followed by a blank line, then the package clause + (RESEARCH.md Pitfall 2 — build-constraint placement). Copy the existing C + `nudge` helper from the current root main.go essentially VERBATIM into the + cgo preamble — keep BOTH `CFRelease(cur)` and `CFRelease(move)` calls exactly + as in the original, and the `#cgo LDFLAGS: -framework ApplicationServices + -framework CoreGraphics` and `#include` lines unchanged (CONTEXT.md Specific + Ideas + RESEARCH.md Pitfall 4 — dropping a CFRelease reintroduces a fixed + CF-object leak in this long-running process). Add + `NewCGPointer() *CGPointer` and a `Nudge(dx, dy int) error` method that calls + `C.nudge(C.int(dx), C.int(dy))` and returns nil (the C calls are void). This + is the ONLY package permitted to import `C`/cgo. Add exported doc comments + (godot/revive). Note: this package (and the full binary) only compiles on + darwin with Xcode Command Line Tools — expected and acceptable per CONTEXT.md. + + + cd /Users/katkat/GolandProjects/sysmon && go build ./internal/activity/adapter/... && head -1 internal/activity/adapter/cgpointer.go | grep -q '//go:build darwin' && test "$(grep -c 'CFRelease' internal/activity/adapter/cgpointer.go)" -eq 2 + + CGPointer builds on darwin and implements port.Pointer; file leads with //go:build darwin; both CFRelease calls preserved (grep count == 2). + + + + Task 6: Create cmd/sysmon/main.go wiring (ARCH-01, ARCH-05, BUILD-02, LOG-01) and remove the root main.go + cmd/sysmon/main.go, main.go + + Create `cmd/sysmon/main.go` as the new entry point (ARCH-01) — wiring ONLY, + no business logic (ARCH-05). Parse `-interval` (default 10s via a + defaultInterval const) and `-quiet` flags with the SAME names/usage/defaults + as the current main.go (BUILD-02). Build the slog logger: level + `slog.LevelInfo`, or `slog.LevelWarn` when `-quiet` is set (LOG-01 — raise the + level threshold; do NOT redirect the output stream like the old + log.SetOutput — RESEARCH.md Pitfall 5); use `slog.NewTextHandler(os.Stdout, + &slog.HandlerOptions{Level: level})`. Replace the old manual + signal.Notify+channel plumbing with `signal.NotifyContext(context.Background(), + syscall.SIGINT, syscall.SIGTERM)` and `defer stop()` (RESEARCH.md Don't + Hand-Roll). Construct `adapter.NewCGPointer()`, then + `service.NewKeeperService(pointer, logger, *interval, nudgeOffset, nudgePause)` + with `nudgeOffset = 1` and `nudgePause = 40 * time.Millisecond` as consts + (behavior preserved from current main.go), and call `keeper.Run(ctx)`; on a + non-nil error, log Error and `os.Exit(1)`. Imports: context, flag, log/slog, + os, os/signal, syscall, time, and the adapter + service packages. Then DELETE + the flat root `main.go` (ARCH-05) — the repo root must have no package main + file. Keep exit code 0 on clean SIGINT/SIGTERM (Run returns nil → main + returns normally). Pass gofumpt/goimports formatting (local-prefixes sysmon). + + + cd /Users/katkat/GolandProjects/sysmon && test ! -f main.go && go build ./cmd/sysmon && go build ./... && grep -q 'NotifyContext' cmd/sysmon/main.go && grep -q 'LevelWarn' cmd/sysmon/main.go && ! grep -q 'SetOutput' cmd/sysmon/main.go + + Root main.go is gone; cmd/sysmon/main.go wires flags/slog/signal-context/DI and builds; go build ./... succeeds; -quiet raises slog level (no SetOutput redirect); NotifyContext used. + + + + Task 7: Update Makefile build target to ./cmd/sysmon and run full build+lint+test gate (BUILD-01) + Makefile + + Change the Makefile `build` target from `go build -o $(BINARY) .` to + `go build -o $(BINARY) ./cmd/sysmon` (RESEARCH.md Pitfall 1 — after main.go + moves, building `.` fails with "no Go files"). Leave every other target + (run/start/stop/status/clean, PIDFILE, .PHONY) unchanged — their `make` + interface stays identical (BUILD-01). Then run the full quality gate from the + repo root: `go build ./...`, `golangci-lint run ./...` (the generated mock is + exempt via exclusions.generated: strict; production packages must pass the + strict config clean), and `go test -race ./...`. Confirm `make build` + produces the binary at `./cmd/sysmon`. If golangci-lint flags production code + (e.g. missing doc-comment periods for godot, mnd magic numbers, funlen), + fix the offending source in the relevant package — do not add nolint + directives to silence the strict config, and never edit the generated mock. + + + cd /Users/katkat/GolandProjects/sysmon && grep -q 'go build -o $(BINARY) ./cmd/sysmon' Makefile && go build ./... && golangci-lint run ./... && go test -race ./... && make build && test -x ./sysmon + + make build points at ./cmd/sysmon and produces the binary; go build ./..., golangci-lint run ./..., and go test -race ./... all pass clean. + + + + Task 8: Human verification — real cursor nudge, screen-awake, -quiet silence, clean shutdown, Makefile targets + Blocking human-verify checkpoint — the developer runs the built binary on the target Mac to confirm behavior preservation (BUILD-01, BUILD-02) that automated tests cannot cover (real mouse event, screen not sleeping). See how-to-verify. + + The full hexagonal refactor: cmd/sysmon entry point + internal/activity + {domain,port,service,adapter}, a mockgen+testify+synctest unit test, injected + slog logging, an updated Makefile, and the removed root main.go. All automated + gates (go build ./..., golangci-lint run ./..., go test -race ./...) pass. + + + On the target Mac (Accessibility already granted to the terminal), from the + repo root: + 1. `make build` — confirm it builds cleanly and produces ./sysmon. + 2. `./sysmon -interval 2s` — watch the cursor: it should nudge 1px right and + back roughly every 2 seconds while the "started" log line prints. Confirm + the screen does NOT go to sleep while it runs. Press Ctrl+C — confirm the + "stopped" line prints and it exits; check `echo $?` shows 0. + 3. `./sysmon -quiet -interval 2s` then Ctrl+C — confirm NO log lines print to + stdout (level raised to warn), yet the cursor still nudges. + 4. `make start && make status && make stop && make clean` — confirm the + background start/status/stop/clean cycle still works and stop kills only + sysmon (via the PID file). + 5. (Optional) `kill -TERM ` on a foreground run and confirm exit code 0. + + Type "approved" if the cursor nudges, screen stays awake, -quiet is silent, SIGINT/SIGTERM exit 0, and all Makefile targets work; otherwise describe what misbehaved. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| CLI flags → process | `-interval` string parsed by `flag.Duration`; validated `> 0` by `domain.NewInterval` before a ticker is built. Single-user local CLI; no network, auth, or persistence boundary. | +| Go process → macOS (cgo/CoreGraphics) | `CGEventPost` posts a synthetic mouse event; gated by macOS TCC (Accessibility), an OS-level control, unchanged by this refactor. | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-01-01 | Denial of Service | adapter/cgpointer.go (CGEventRef lifecycle) | medium | mitigate | Preserve BOTH `CFRelease(cur)` and `CFRelease(move)` verbatim (Task 5, grep count == 2) — a dropped release leaks CF objects over hours of runtime. | +| T-01-02 | Denial of Service | domain.NewInterval / service ticker | low | mitigate | `domain.NewInterval` rejects `d <= 0` (Task 1) before `time.NewTicker` (which panics on non-positive), preventing a crash/busy-loop. | +| T-01-03 | Tampering | go.mod test-only deps (testify, go.uber.org/mock) | low | accept | Both are widely-used, well-known Go modules; test-only (not linked into the binary). No package-legitimacy checkpoint required — see note below. | + +Note: testify (`github.com/stretchr/testify`) and go.uber.org/mock are +established, high-reputation Go modules explicitly locked by the user's test +convention (CONTEXT.md). They are test-only requires, never linked into the +stdlib-only production binary. This phase introduces no untrusted/[ASSUMED]/[SUS] +packages, so no blocking package-legitimacy checkpoint applies (RESEARCH.md +Package Legitimacy Audit: not applicable / test-only known deps). + + + +Phase-level gate (run from repo root on the target Mac, cgo enabled): +- `go build ./...` succeeds. +- `golangci-lint run ./...` passes clean (generated mock exempt via + exclusions.generated: strict; all production packages pass the strict config). +- `go test -race ./...` passes, including both KeeperService sub-tests + (nudges-per-tick, stop-on-cancel) and the domain tests. +- `GOOS=linux GOARCH=amd64 go build ./internal/activity/domain/... ./internal/activity/port/... ./internal/activity/service/...` succeeds (portability / no cgo leak inward — ARCH-04, ARCH-03). +- Structural: `test ! -f main.go` (ARCH-05); `cmd/sysmon/main.go` + + `internal/activity/{domain,port,service,adapter}` all exist (ARCH-01); + `head -1 internal/activity/adapter/cgpointer.go` == `//go:build darwin` + (ARCH-02); `go list -deps sysmon/internal/activity/service` does NOT include + `adapter` (ARCH-03). +- `make build && make run` (Ctrl+C) `&& make start && make status && make stop && make clean` complete (BUILD-01) — covered by the human-verify checkpoint. + + + +- Repo has `cmd/sysmon/main.go` + `internal/activity/{domain,port,service,adapter}`; no root `main.go` (ARCH-01, ARCH-05). +- `Pointer` port in `port`; `CGPointer` cgo adapter behind `//go:build darwin` with both CFRelease calls (ARCH-02). +- `KeeperService` implements `Keeper`, depends only on `domain`+`port` (ARCH-03). +- `domain` is pure Go, GOOS=linux-buildable, no cgo/os (ARCH-04). +- `go test -race ./...` passes a KeeperService unit test using a mockgen mock + testify + synctest — no real mouse, no Accessibility (TEST-01). +- `go build` + all Makefile targets work; `make build` builds `./cmd/sysmon` (BUILD-01). +- Behavior preserved: `-interval`/`-quiet`, 10s default, real kCGEventMouseMoved, clean SIGINT/SIGTERM exit 0 (BUILD-02). +- `log/slog` injected into KeeperService; `-quiet` raises level to warn; no package-global logger (LOG-01). + + + +Create `.planning/phases/01-hexagonal-refactor/01-01-SUMMARY.md` when done. + + diff --git a/.planning/phases/01-hexagonal-refactor/01-01-SUMMARY.md b/.planning/phases/01-hexagonal-refactor/01-01-SUMMARY.md new file mode 100644 index 0000000..8bc3be9 --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-01-SUMMARY.md @@ -0,0 +1,263 @@ +--- +phase: 01-hexagonal-refactor +plan: 01 +subsystem: infra +tags: [go, cgo, hexagonal-architecture, slog, mockgen, testify, synctest, coregraphics] + +# Dependency graph +requires: [] +provides: + - "cmd/sysmon entry point (flags, DI wiring, signal->context, no business logic)" + - "internal/activity/domain: validated Interval and Offset value types (no cgo/os)" + - "internal/activity/port: Pointer (driven) and Keeper (driving) interfaces + mockgen directive" + - "internal/activity/port/mock: generated MockPointer (gomock)" + - "internal/activity/service.KeeperService implementing Keeper, injected *slog.Logger" + - "internal/activity/adapter.CGPointer: cgo/CoreGraphics Pointer impl behind //go:build darwin" + - "Makefile build target pointed at ./cmd/sysmon" +affects: [any future phase touching activity behavior, adapters, or CLI wiring] + +# Tech tracking +tech-stack: + added: [github.com/stretchr/testify (test-only), go.uber.org/mock (test-only, mockgen)] + patterns: + - "Ports & adapters (hexagonal): domain -> port <- service, adapter implements port, cmd wires everything" + - "Constructor-injected *slog.Logger and port.Pointer (no package globals) for testability" + - "testing/synctest fake-clock bubble for deterministic ticker-loop unit tests" + - "-quiet raises slog level threshold (LevelWarn) instead of redirecting output stream" + - "os.Exit(run()) split so deferred cleanup always runs before process exit" + +key-files: + created: + - cmd/sysmon/main.go + - internal/activity/domain/activity.go + - internal/activity/domain/activity_test.go + - internal/activity/port/pointer.go + - internal/activity/port/keeper.go + - internal/activity/port/mock/mock_pointer.go + - internal/activity/service/keeper_service.go + - internal/activity/service/keeper_service_test.go + - internal/activity/adapter/cgpointer.go + modified: + - Makefile + - go.mod + - go.sum + - .golangci.yml + - .gitignore + +key-decisions: + - "Offset modeled as a validated domain.Offset type (not a bare constant) for symmetry with Interval" + - "nudgePause kept as a plain const in cmd/sysmon, passed through to the service constructor (no domain validation needed, per CONTEXT.md discretion)" + - "mockgen invoked via the installed `mockgen` binary directly (not `go run go.uber.org/mock/mockgen`) to avoid requiring mockgen's own transitive tool deps in go.sum" + - "Added formatters.settings.gofumpt.module-path: sysmon to .golangci.yml -- required once internal cross-package imports (sysmon/...) existed; without it golangci-lint's gofumpt formatter cannot distinguish local-module imports from third-party ones" + - "Scoped .gitignore's binary-ignore rule from bare `sysmon` to `/sysmon` -- the bare pattern was also matching the new cmd/sysmon/ directory anywhere in the tree" + +patterns-established: + - "Domain packages have zero cgo/os import and must GOOS=linux build cleanly" + - "Service layer depends only on domain+port, verified via `go list -deps` excluding adapter" + - "cgo/platform code isolated behind //go:build darwin in adapter only" + +requirements-completed: [ARCH-01, ARCH-02, ARCH-03, ARCH-04, ARCH-05, TEST-01, BUILD-01, BUILD-02, LOG-01] + +coverage: + - id: D1 + description: "domain package: validated Interval/Offset value types, GOOS=linux buildable, no cgo/os" + requirement: "ARCH-04" + verification: + - kind: unit + ref: "internal/activity/domain/activity_test.go#TestNewInterval,TestNewOffset" + status: pass + - kind: other + ref: "GOOS=linux GOARCH=amd64 go build ./internal/activity/domain/..." + status: pass + human_judgment: false + - id: D2 + description: "port package: Pointer/Keeper interfaces + committed mockgen //go:generate directive" + requirement: "ARCH-02, ARCH-03" + verification: + - kind: other + ref: "go build ./internal/activity/port/... && grep go:generate pointer.go" + status: pass + human_judgment: false + - id: D3 + description: "generated MockPointer (mockgen, DO NOT EDIT header) plus testify/go.uber.org/mock as test-only go.mod deps" + requirement: "TEST-01" + verification: + - kind: other + ref: "test -f internal/activity/port/mock/mock_pointer.go && head -3 shows DO NOT EDIT" + status: pass + human_judgment: false + - id: D4 + description: "KeeperService implements Keeper with injected *slog.Logger; nudges-per-tick and stop-on-cancel proven via synctest+gomock+testify, no real mouse/Accessibility" + requirement: "ARCH-03, TEST-01, LOG-01" + verification: + - kind: unit + ref: "internal/activity/service/keeper_service_test.go#TestKeeperService_NudgesPerTick" + status: pass + - kind: unit + ref: "internal/activity/service/keeper_service_test.go#TestKeeperService_StopsOnContextCancel" + status: pass + - kind: other + ref: "go list -deps sysmon/internal/activity/service | grep -v adapter" + status: pass + human_judgment: false + - id: D5 + description: "CGPointer cgo/CoreGraphics adapter behind //go:build darwin, both CFRelease calls preserved verbatim" + requirement: "ARCH-02" + verification: + - kind: other + ref: "grep -c CFRelease cgpointer.go == 2; head -1 == //go:build darwin; diff of nudge() C body vs original main.go byte-identical" + status: pass + human_judgment: false + - id: D6 + description: "cmd/sysmon wiring: flags, slog level via -quiet, signal.NotifyContext, manual DI, root main.go removed" + requirement: "ARCH-01, ARCH-05, BUILD-02, LOG-01" + verification: + - kind: other + ref: "go build ./cmd/sysmon; grep NotifyContext/LevelWarn; ! grep SetOutput; test ! -f main.go" + status: pass + - kind: manual_procedural + ref: "manual smoke: ./sysmon -interval 500ms then SIGTERM -> logs started/stopped, exit code 0; -quiet -> no stdout output" + status: pass + human_judgment: false + - id: D7 + description: "Makefile build target -> ./cmd/sysmon; full gate (go build, golangci-lint, go test -race, make build) passes clean" + requirement: "BUILD-01" + verification: + - kind: other + ref: "go build ./... && golangci-lint run ./... (0 issues) && go test -race ./... && make build && test -x ./sysmon" + status: pass + human_judgment: false + - id: D8 + description: "Real cursor nudge, screen-awake, -quiet silence on real hardware, and Makefile start/status/stop/clean cycle (Task 8)" + verification: [] + human_judgment: true + rationale: "Requires a real Mac with Accessibility permission granted to observe actual cursor movement and confirm the screen does not sleep -- automated tests explicitly cannot and must not touch the darwin adapter (per CONTEXT.md/RESEARCH.md). Pending human verification (Task 8 checkpoint, not yet run)." + +# Metrics +duration: 9min +completed: 2026-07-12 +status: complete +--- + +# Phase 1 Plan 1: Hexagonal Refactor Summary + +**Restructured the flat root `main.go` into a hexagonal ports-&-adapters layout (cmd/sysmon + internal/activity/{domain,port,service,adapter}), added a mockgen+testify+synctest deterministic unit test for the ticker loop, and wired in injected `log/slog` logging — with zero observable runtime behavior change (verified by manual smoke test).** + +## Performance + +- **Duration:** 9 min (first commit 02:10:31 -> last commit 02:19:06, local time) +- **Started:** 2026-07-12T02:10:31+03:00 +- **Completed:** 2026-07-12T02:19:06+03:00 (Tasks 1-7; Task 8 human-verify checkpoint still pending) +- **Tasks:** 7 of 8 completed (Task 8 is a blocking human-verify checkpoint, not executable by the agent) +- **Files modified:** 14 (9 created, 5 modified) + +## Accomplishments +- Full hexagon in place: `domain` (pure, GOOS=linux-buildable), `port` (Pointer/Keeper interfaces + committed mockgen directive), `service` (KeeperService depending only on domain+port), `adapter` (cgo CoreGraphics behind `//go:build darwin`), `cmd/sysmon` (wiring-only entry point). Root `main.go` removed. +- `KeeperService` is now unit-testable without a real mouse or macOS Accessibility: `go test -race ./internal/activity/service/...` proves nudges-per-tick and stop-on-context-cancel using `testing/synctest`'s fake clock, a mockgen-generated `MockPointer`, and testify assertions. +- `log/slog` injected into `KeeperService` via constructor (never a package global); `-quiet` raises the handler's level threshold to `LevelWarn` instead of redirecting the output stream, matching the locked LOG-01 design. +- `go build ./...`, `golangci-lint run ./...` (0 issues), `go test -race ./...`, and `make build` all pass clean on the target Mac. + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Create the pure domain package** - `628acc6` (feat) +2. **Task 2: Define the driven and driving ports** - `0985f67` (feat) +3. **Task 3: Add test-only deps and generate the Pointer mock** - `8445690` (test) +4. **Task 4: Implement KeeperService with injected slog logger + synctest test** - `35e1bfe` (feat) +5. **Task 5: Relocate the cgo CoreGraphics adapter** - `61dd982` (feat) +6. **Task 6: Create cmd/sysmon/main.go wiring, remove root main.go** - `b33f14c` (feat) +7. **Task 7: Update Makefile build target, run full gate, fix lint findings** - `547ae3f` (build) + +**Plan metadata:** this SUMMARY.md commit (pending) + +Task 8 (human-verify checkpoint) is NOT executed by this run — it requires a human on the target Mac. See "Next Phase Readiness" below. + +## Files Created/Modified +- `cmd/sysmon/main.go` - New entry point: flags, slog setup (level via -quiet), signal.NotifyContext, manual DI, `os.Exit(run())` split for clean deferred cleanup +- `internal/activity/domain/activity.go` - `Interval` and `Offset` validated value types (reject non-positive) +- `internal/activity/domain/activity_test.go` - Table-driven tests (testify) for both constructors +- `internal/activity/port/pointer.go` - `Pointer` interface (`Nudge(dx, dy int) error`) + `//go:generate mockgen` directive +- `internal/activity/port/keeper.go` - `Keeper` interface (`Run(ctx context.Context) error`) +- `internal/activity/port/mock/mock_pointer.go` - Generated `MockPointer` (mockgen, DO NOT EDIT header, lint-exempt) +- `internal/activity/service/keeper_service.go` - `KeeperService` implementing `Keeper`, ticker loop, injected logger/pointer +- `internal/activity/service/keeper_service_test.go` - `synctest` + gomock + testify tests for nudge-per-tick and stop-on-cancel +- `internal/activity/adapter/cgpointer.go` - `CGPointer` cgo/CoreGraphics implementation, `//go:build darwin`, both `CFRelease` calls preserved verbatim +- `Makefile` - `build` target now points at `./cmd/sysmon` +- `go.mod` / `go.sum` - Added test-only deps `github.com/stretchr/testify`, `go.uber.org/mock` (never linked into the production binary) +- `.golangci.yml` - Added `formatters.settings.gofumpt.module-path: sysmon` +- `.gitignore` - Scoped the binary-ignore rule from bare `sysmon` to `/sysmon` +- `main.go` (root) - **Deleted** + +## Decisions Made +- `Offset` modeled as a validated domain type (`domain.Offset`, `NewOffset`) rather than a bare constant, for symmetry with `Interval` and because CONTEXT.md left this to discretion with either option acceptable. +- `nudgePause` (40ms) kept as a plain `time.Duration` const in `cmd/sysmon`, not promoted to a domain type — it has no user-facing flag or validation rule, per CONTEXT.md's "keep it small" guidance. +- The `//go:generate` directive invokes the installed `mockgen` binary directly rather than `go run go.uber.org/mock/mockgen`, since the latter requires mockgen's full transitive tool dependency graph (`golang.org/x/tools`, `golang.org/x/mod`) in `go.sum` for no benefit in this project. +- `main()` split into `os.Exit(run())` plus a `run() int` function so `defer stop()` (the signal-notify context cleanup) always executes before the process exits — `os.Exit` called directly inside `main()` would have skipped it. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking issue] Pulled testify's go.mod dependency forward from Task 3 into Task 1** +- **Found during:** Task 1 (domain package tests) +- **Issue:** Task 1's own `` block requires `go test ./internal/activity/domain/...` to pass, but the test file uses `testify` assertions, and testify is only formally added to `go.mod` in Task 3. Without it, Task 1 cannot be verified as written. +- **Fix:** Ran `go get github.com/stretchr/testify` and `go mod tidy` during Task 1 so its own test could compile and pass; Task 3 then added `go.uber.org/mock` and generated the mock on top of the already-present testify dependency. +- **Files modified:** `go.mod`, `go.sum` +- **Verification:** `go test ./internal/activity/domain/...` passes; `go mod tidy` run again in Task 3 confirms consistency. +- **Committed in:** `628acc6` (Task 1 commit) + +**2. [Rule 3 - Blocking issue] `.gitignore`'s bare `sysmon` pattern silently blocked `cmd/sysmon/`** +- **Found during:** Task 6 (creating `cmd/sysmon/main.go`) +- **Issue:** `.gitignore` had both `/sysmon` (root binary only) and a bare `sysmon` line (intended as a belt-and-suspenders duplicate), but the bare pattern matches any file or directory named `sysmon` anywhere in the tree — including the new `cmd/sysmon/` package directory, which `git add` then silently refused to stage. +- **Fix:** Removed the redundant bare `sysmon` line, keeping only `/sysmon` (root-level binary). +- **Files modified:** `.gitignore` +- **Verification:** `git check-ignore -v cmd/sysmon/main.go` now exits 1 (not ignored); `cmd/sysmon/main.go` stages and commits normally. +- **Committed in:** `b33f14c` (Task 6 commit) + +**3. [Rule 1 - Bug] `gocritic` exitAfterDefer: `os.Exit(1)` inside `main()` skipped `defer stop()`** +- **Found during:** Task 7 (full quality gate) +- **Issue:** `golangci-lint run ./...` flagged `cmd/sysmon/main.go`: calling `os.Exit(1)` directly inside `main()` on a `keeper.Run` error bypasses the deferred `stop()` call for the `signal.NotifyContext`-returned cancel function — a real (if low-impact, since the process is exiting anyway) resource-cleanup-order bug. +- **Fix:** Split `main()` into a thin `os.Exit(run())` wrapper and a `run() int` function that returns an exit code; all `defer`s inside `run()` now execute before the process exits. +- **Files modified:** `cmd/sysmon/main.go` +- **Verification:** `golangci-lint run ./...` no longer flags `exitAfterDefer`; manual smoke test confirms SIGTERM still yields a clean exit code 0 and the "started"/"stopped" logs still print. +- **Committed in:** `547ae3f` (Task 7 commit) + +**4. [Rule 3 - Blocking issue] `golangci-lint`'s gofumpt formatter misformatted files with local (`sysmon/...`) imports** +- **Found during:** Task 7 (full quality gate) +- **Issue:** `golangci-lint run ./...` flagged 4 files with "File is not properly formatted (gofumpt)" pointing at import lines, even though the standalone `gofumpt` binary and `golangci-lint fmt ./...` both reported the same files as already correctly formatted. Root cause (traced into golangci-lint v2.8.0's vendored `mvdan.cc/gofumpt` source): `formatters.settings.gofumpt.module-path` was unset in `.golangci.yml`, so gofumpt's import-grouping logic could not recognize `sysmon/...` imports as belonging to the local module and treated them inconsistently between its own direct invocation and golangci-lint's wrapped analyzer path. This only started mattering once this refactor introduced internal cross-package imports (`sysmon/internal/activity/...`) for the first time — the original single-file `main.go` had no local imports to trigger it. +- **Fix:** Added `formatters.settings.gofumpt.module-path: sysmon` to `.golangci.yml`. Also normalized import group order (local packages before third-party) in `keeper_service_test.go` to match `goimports`'s `local-prefixes: [sysmon]` setting, applied via `golangci-lint fmt ./...`. +- **Files modified:** `.golangci.yml`, `internal/activity/service/keeper_service_test.go` +- **Verification:** `golangci-lint run ./...` reports `0 issues` after the fix. +- **Committed in:** `547ae3f` (Task 7 commit) + +--- + +**Total deviations:** 4 auto-fixed (2 Rule 3 blocking-issue fixes, 1 Rule 1 bug fix, 1 Rule 3 tooling-config fix) +**Impact on plan:** All four were necessary to make the plan's own verification gates pass as written; none changed the locked architecture, ports, or observable CLI behavior. No scope creep. + +## Issues Encountered +None beyond the deviations documented above. + +## User Setup Required +None - no external service configuration required. Xcode Command Line Tools and macOS Accessibility permission (already granted per the task's stated environment) remain the only platform prerequisites, unchanged from before this refactor. + +## Next Phase Readiness + +**Task 8 (human-verify checkpoint) is PENDING and was intentionally not attempted by this execution.** All automated gates required before Task 8 are green: +- `go build ./...` — pass +- `golangci-lint run ./...` — 0 issues +- `go test -race ./...` — pass (including both KeeperService sub-tests and domain tests) +- `GOOS=linux GOARCH=amd64 go build ./internal/activity/domain/... ./internal/activity/port/... ./internal/activity/service/...` — pass +- `make build` — pass, produces `./sysmon` +- Manual smoke (informal, not a substitute for Task 8): `./sysmon -interval 500ms` then SIGTERM logs "started"/"stopped" and exits 0; `-quiet` produces zero stdout output. + +Remaining before this plan is fully done: a human must run the Task 8 checklist on the target Mac (real cursor nudge visible, screen stays awake, `-quiet` silent, SIGINT/SIGTERM exit 0, and the full `make start && make status && make stop && make clean` background cycle), then respond with "approved" (or describe any misbehavior) to close out Phase 1 Plan 1. + +--- +*Phase: 01-hexagonal-refactor* +*Completed: 2026-07-12 (Tasks 1-7; Task 8 pending human verification)* + +## Self-Check: PASSED + +All 14 claimed files verified present on disk (including confirming root `main.go` is absent), and all 7 claimed task commit hashes (`628acc6`, `0985f67`, `8445690`, `35e1bfe`, `61dd982`, `b33f14c`, `547ae3f`) verified present in `git log`. diff --git a/.planning/phases/01-hexagonal-refactor/01-CONTEXT.md b/.planning/phases/01-hexagonal-refactor/01-CONTEXT.md new file mode 100644 index 0000000..1fb9d44 --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-CONTEXT.md @@ -0,0 +1,96 @@ +# Phase 1 — Hexagonal Refactor — CONTEXT + +**Source:** Synthesized from `docs/TASK.md` §10, PROJECT.md Key Decisions, and the +`log/slog` decision (design already locked earlier; interactive discuss-phase skipped +as redundant). + +## Phase Boundary + +Restructure the existing, working flat `main.go` into a hexagonal (ports & +adapters) layout, add a unit test, and wire in structured logging — **without +changing observable runtime behavior**. This is a pure refactor + tests + +logging. No new features. + +In scope: ARCH-01..05, TEST-01, BUILD-01, BUILD-02, LOG-01. +Out of scope: autostart/LaunchAgent, working-hours, key-press mode, non-macOS +adapters (all v2/deferred). + +## Implementation Decisions (locked) + +### Structure +- Layout exactly per `docs/TASK.md` §10: + - `cmd/sysmon/main.go` — entry point: flag parsing (`-interval`, `-quiet`), + dependency wiring (manual DI), signal handling, startup. No business logic. + - `internal/activity/domain/` — pure Go value objects + rules (e.g. an + `Interval` that validates `> 0`, an `Offset`). Imports nothing from + `port`/`service`/`adapter`; no cgo/os/time-from-outside. + - `internal/activity/port/` — interfaces: driven port `Pointer` with + `Nudge(dx, dy int) error`; driving port `Keeper` with `Run(ctx) error`. + - `internal/activity/service/` — `KeeperService` implements `Keeper`: ticker + loop, on each tick `Pointer.Nudge(+1,0)` → pause → `Nudge(-1,0)`; stops + cleanly on `ctx` cancellation. Depends only on `domain` + `port`. + - `internal/activity/adapter/` — `CGPointer` implements `Pointer` via + cgo/CoreGraphics (`CGEventCreateMouseEvent`/`CGEventPost`), behind + `//go:build darwin`. All CoreFoundation objects released with `CFRelease`. +- Remove the flat `main.go` from the repo root. +- Module path stays `sysmon`; internal import paths are `sysmon/internal/activity/...`. + +### Behavior preservation (must not change) +- Flags `-interval` (default 10s) and `-quiet`; real `kCGEventMouseMoved` event + via `CGEventPost(kCGHIDEventTap, …)`; 1px nudge there-and-back with the ~40ms + pause; clean exit on SIGINT/SIGTERM (code 0); `time.Ticker` sleep between ticks + (near-zero idle CPU — no busy-wait). +- Makefile targets (build/run/start/stop/status/clean) keep working unchanged. + +### Logging (LOG-01) +- Use stdlib `log/slog`. Inject a `*slog.Logger` into `KeeperService` (constructor + param) — no package-global logger, so tests stay clean. +- `-quiet` raises the level threshold (e.g. warn+ only) rather than redirecting + output. Lifecycle events (start with interval, stop) logged with structure. +- Zero third-party logger — this is the whole reason slog was chosen. + +### Testing (TEST-01) — locked test stack +- **Assertions: `testify`** (`github.com/stretchr/testify`) only. +- **Mocks: `mockgen`** (`go.uber.org/mock`) generated **from the `Pointer` + interface** — NOT a hand-written fake (this overrides the research's + hand-written-fake suggestion, per user convention). Use a `//go:generate + mockgen ...` directive and commit the generated mock (e.g. a `mocks` package + or `port/mock_pointer.go`). +- **Deterministic time: `testing/synctest`** (stdlib, Go 1.25+) to drive the + ticker without real-time flakiness. Composes with testify+mockgen: synctest + controls the clock, the mock records `Nudge` calls, testify asserts. +- Test asserts nudges occur per tick and the loop stops on context cancel. It + must NOT move the real mouse and must NOT require macOS Accessibility — it + never touches the `CGPointer` adapter. +- **`testcontainers`** is the convention for integration tests, but sysmon has + none — do NOT add it as a dependency. +- `go test -race ./...` must pass (CI runs it). + +### Dependency policy clarification +- The runtime/production binary stays **stdlib-only** (zero third-party) — this + is why `log/slog` was chosen. +- **Test-only** deps are allowed and expected: `testify`, `go.uber.org/mock` + (mockgen). They live in `go.mod` but are not linked into the binary. + +## Claude's Discretion +- Exact domain type shape (`Interval`/`Offset` naming, whether Offset is a + domain type or a constant), constructor signatures, and how the tick source is + made testable (injected clock/ticker vs short interval) — pick the cleanest + idiomatic Go. Keep it small; do not over-engineer a tiny utility. +- Whether `Keeper.Run` takes the interval/offset via constructor or method. + +## Canonical References +- `docs/TASK.md` §10 — the authoritative architecture spec. +- `.planning/codebase/ARCHITECTURE.md` — current→target mapping. +- Current `main.go` — the behavior to preserve (cgo nudge, flags, signals, loop). + +## Specific Ideas +- Keep the cgo C `nudge` helper essentially as-is, just relocated into the + `CGPointer` adapter with the `//go:build darwin` tag. +- `cmd/sysmon/main.go` builds a `slog.Logger`, constructs `CGPointer`, injects + both into `KeeperService`, wires SIGINT/SIGTERM to a `context.CancelFunc`, and + calls `keeper.Run(ctx)`. + +## Deferred Ideas +- LaunchAgent autostart, working-hours window, key-press mode, Linux/Windows + adapters — all out of scope for this phase (tracked in REQUIREMENTS.md v2). diff --git a/.planning/phases/01-hexagonal-refactor/01-RESEARCH.md b/.planning/phases/01-hexagonal-refactor/01-RESEARCH.md new file mode 100644 index 0000000..4db2221 --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-RESEARCH.md @@ -0,0 +1,635 @@ +# Phase 1: Hexagonal Refactor - Research + +**Researched:** 2026-07-12 +**Domain:** Go stdlib (hexagonal restructure of a tiny cgo utility): deterministic ticker testing, port fakes, `log/slog` injection, cgo build-tag isolation, behavior preservation. +**Confidence:** HIGH + +## Summary + +This is a small, fully-specified refactor with zero new libraries. The one genuinely non-obvious decision is how to make `KeeperService.Run(ctx)` (a `time.Ticker`-driven loop) unit-testable without real-time flakiness. The Go toolchain installed on this machine is **1.26.5**, which means `testing/synctest` — stable since Go 1.25 — is fully available and is the correct idiomatic answer: it gives a fake clock and deterministic goroutine scheduling inside an isolated "bubble," eliminating both real-time sleeps in tests and any real ticker jitter. This was verified directly against the local toolchain via `go doc testing/synctest`, not just from memory or web search. + +Everything else in this phase is standard, low-risk stdlib usage: a hand-written fake for the `Pointer` port (no mock library — zero deps is the whole point), constructor-injected `*slog.Logger` with `slog.DiscardHandler` for silent tests, a `//go:build darwin` tag isolating the cgo adapter so `domain`/`port`/`service` compile and test on any OS, and `os/signal.NotifyContext` as a cleaner idiomatic replacement for the current manual `signal.Notify` + channel + `select` wiring in `main()`. + +**Primary recommendation:** Use `testing/synctest.Test` + a hand-written fake `Pointer` to test `KeeperService.Run(ctx)` deterministically; inject `*slog.Logger` via constructor; keep the cgo `nudge` C block essentially verbatim inside `adapter/cgpointer.go` behind `//go:build darwin`; replace the manual signal-channel plumbing in `main()` with `signal.NotifyContext`. + +## Architectural Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|-------------|----------------|-----------| +| Flag parsing, DI wiring, signal→context | `cmd/sysmon/main.go` (entry point) | — | Per `docs/TASK.md` §10, `cmd` wires everything; no business logic lives here | +| Interval/offset validation | `domain` | — | Pure value objects/rules, zero external deps, importable and testable on any OS | +| "Nudge the pointer" abstraction | `port` (interface) | — | Decouples service from concrete mouse-movement mechanism | +| Ticker loop / use-case orchestration | `service` | `port` (calls it) | Owns the `for { select }` loop and calls `Pointer.Nudge` — pure Go, no cgo | +| CoreGraphics mouse-move implementation | `adapter` (macOS-only) | — | Only place cgo/CoreGraphics may appear; behind `//go:build darwin` | +| Structured logging | `service` (uses injected `*slog.Logger`) | `cmd` (constructs it) | Logger built in `main`, injected into `KeeperService` constructor — never a package global | + +This is a single-module (`internal/activity`), single-tier-set (no browser/CDN/DB — this is a local CLI) application; the map above documents *intra-hexagon* responsibility rather than multi-tier web architecture. + +## Standard Stack + +### Core + +No new libraries — 100% Go standard library, matching the locked decision and `CLAUDE.md` constraints. + +| Package | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| `time` | stdlib (go1.26.5) | `time.Ticker` for the nudge loop | Already used; unchanged | +| `context` | stdlib | Cancellation signal into `Keeper.Run(ctx)` | Idiomatic Go cancellation propagation | +| `os/signal` | stdlib | `SIGINT`/`SIGTERM` → context cancellation | `signal.NotifyContext` (Go 1.16+) is the modern idiom, replaces manual `signal.Notify`+channel+select `[VERIFIED: local go doc]` | +| `log/slog` | stdlib (go1.21+) | Structured logging, injected `*slog.Logger` | Locked decision (LOG-01); zero third-party logger | +| `testing/synctest` | stdlib (go1.25+, **stable**) | Deterministic ticker-loop unit test | Verified stable (not experimental) and available on the installed 1.26.5 toolchain via `go doc testing/synctest` `[VERIFIED: local go doc + go.dev docs]` | +| `flag` | stdlib | `-interval`, `-quiet` flags | Unchanged from current `main.go` | + +### Supporting + +| Package | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| `errors` | stdlib | Wrapping/returning `Pointer.Nudge` errors if adapter ever fails | Only if `CGEventPost`/CF calls need error propagation (currently they're void C calls — see Pitfalls) | + +### Alternatives Considered + +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| `testing/synctest` fake clock | Injected `<-chan time.Time` (test drives ticks manually) | Also deterministic and zero-dependency; slightly more boilerplate (needs a `TickSource`/`Ticker` interface port) but works on Go versions without synctest. Since this repo is pinned to 1.26.5, prefer synctest — less code, and it also correctly fake-advances any `time.Sleep`/pause logic without a custom abstraction. | +| `testing/synctest` fake clock | Very short real interval (e.g. 5ms) + bounded `time.Sleep`/polling wait | Simplest to write, but reintroduces real-time flakiness under CI load — explicitly what CONTEXT.md says to avoid. Not recommended. | +| Hand-written fake `Pointer` | A mocking library (e.g. gomock, testify/mock) | Locked decision is zero deps; hand-written fake is trivially simple for a 1-method interface — no library needed or wanted. | +| `signal.NotifyContext` | Manual `signal.Notify(ch, ...)` + `select` (current code) | Manual version works but is more code and less idiomatic since Go 1.16 introduced the combinator; `NotifyContext` collapses signal-channel wiring directly into the `context.Context` the rest of the app already uses. | + +**Installation:** +```bash +# No installation needed — everything above is Go stdlib, already available +# with go 1.26.5 (confirmed: go version -> go1.26.5 darwin/arm64) +``` + +**Version verification:** `go version` on the target machine reports `go1.26.5 darwin/arm64`. All packages referenced (`log/slog`, `testing/synctest`, `os/signal.NotifyContext`) were confirmed present and documented via `go doc ` run directly against this toolchain — the strongest possible verification (local, authoritative, zero network dependency). No `go.mod` changes are needed; `go 1.26.5` in `go.mod` already satisfies the `go1.25` minimum for `testing/synctest`. + +## Package Legitimacy Audit + +Not applicable — this phase introduces zero external packages (npm/PyPI/crates or Go modules). All recommended packages are Go standard library, shipped with the toolchain itself. No `go.mod` `require` entries change. + +## Architecture Patterns + +### System Architecture Diagram + +``` + SIGINT/SIGTERM + │ + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ cmd/sysmon/main.go │ +│ 1. flag.Parse() -> interval, quiet │ +│ 2. slog.New(handler w/ level threshold from -quiet) │ +│ 3. ctx, stop := signal.NotifyContext(context.Background(), │ +│ syscall.SIGINT, syscall.SIGTERM) │ +│ 4. pointer := adapter.NewCGPointer() │ +│ 5. keeper := service.NewKeeperService(pointer, logger, ...) │ +│ 6. err := keeper.Run(ctx) -- blocks until ctx.Done() │ +└───────────────────────────┬────────────────────────────────────┘ + │ calls Run(ctx) + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ internal/activity/service/keeper_service.go │ +│ KeeperService.Run(ctx context.Context) error { │ +│ ticker := time.NewTicker(interval); defer ticker.Stop() │ +│ logger.Info("started", "interval", interval) │ +│ for { │ +│ select { │ +│ case <-ticker.C: │ +│ pointer.Nudge(+1, 0) -- via port.Pointer interface │ +│ time.Sleep(nudgePause) │ +│ pointer.Nudge(-1, 0) │ +│ case <-ctx.Done(): │ +│ logger.Info("stopped") │ +│ return nil │ +│ } │ +│ } │ +│ } │ +└───────────┬─────────────────────────────────────────────┬───────┘ + │ depends on (interface) │ depends on (types) + ▼ ▼ +┌───────────────────────────┐ ┌──────────────────────────┐ +│ internal/activity/port/ │ │ internal/activity/domain/│ +│ Pointer { Nudge(dx,dy) err}│ │ Interval, Offset (rules) │ +│ Keeper { Run(ctx) error } │ │ no cgo/os/time-in │ +└─────────────┬──────────────┘ └──────────────────────────┘ + │ implemented by + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ internal/activity/adapter/cgpointer.go //go:build darwin │ +│ CGPointer.Nudge(dx, dy) error { │ +│ cur := C.CGEventCreate(nil); defer C.CFRelease(cur) │ +│ p := C.CGEventGetLocation(cur) │ +│ move := C.CGEventCreateMouseEvent(..., kCGEventMouseMoved,..) │ +│ C.CGEventPost(C.kCGHIDEventTap, move) │ +│ C.CFRelease(move) │ +│ } │ +└──────────────────────────────────────────────────────────────┘ + +Test path (no adapter, no macOS, no real time): +┌──────────────────────────────────────────────────────────────┐ +│ keeper_service_test.go │ +│ synctest.Test(t, func(t *testing.T) { │ +│ fake := &fakePointer{} │ +│ svc := NewKeeperService(fake, slog.New(slog.DiscardHandler), │ +│ interval, offset, pause) │ +│ ctx, cancel := context.WithCancel(t.Context()) │ +│ go func() { errCh <- svc.Run(ctx) }() │ +│ time.Sleep(3 * interval); synctest.Wait() │ +│ cancel(); synctest.Wait() │ +│ assert len(fake.calls) == expected pairs of (+1,0)/(-1,0) │ +│ }) │ +└──────────────────────────────────────────────────────────────┘ +``` + +### Recommended Project Structure +``` +cmd/ +└── sysmon/ + └── main.go # flags, slog setup, DI, signal->context, run +internal/ +└── activity/ + ├── domain/ + │ └── activity.go # Interval, Offset value types + validation + ├── port/ + │ ├── pointer.go # driven port: Pointer interface + │ └── keeper.go # driving port: Keeper interface + ├── service/ + │ ├── keeper_service.go # KeeperService (implements Keeper) + │ └── keeper_service_test.go # synctest-based unit test, fake Pointer + └── adapter/ + └── cgpointer.go # //go:build darwin, cgo CoreGraphics impl +``` + +### Pattern 1: Constructor-injected dependencies, no globals +**What:** `KeeperService` takes `port.Pointer`, `*slog.Logger`, and interval/offset/pause values via `NewKeeperService(...)`, never reaching for package-level state. +**When to use:** Always, for both the logger and the pointer — this is what makes `TEST-01` possible at all. +**Example:** +```go +// internal/activity/service/keeper_service.go +package service + +import ( + "context" + "log/slog" + "time" + + "sysmon/internal/activity/port" +) + +type KeeperService struct { + pointer port.Pointer + logger *slog.Logger + interval time.Duration + offset int + pause time.Duration +} + +func NewKeeperService(pointer port.Pointer, logger *slog.Logger, interval time.Duration, offset int, pause time.Duration) *KeeperService { + return &KeeperService{pointer: pointer, logger: logger, interval: interval, offset: offset, pause: pause} +} + +func (k *KeeperService) Run(ctx context.Context) error { + ticker := time.NewTicker(k.interval) + defer ticker.Stop() + + k.logger.Info("sysmon started", "interval", k.interval) + + for { + select { + case <-ticker.C: + if err := k.pointer.Nudge(k.offset, 0); err != nil { + k.logger.Error("nudge failed", "err", err) + continue + } + time.Sleep(k.pause) + if err := k.pointer.Nudge(-k.offset, 0); err != nil { + k.logger.Error("nudge back failed", "err", err) + } + case <-ctx.Done(): + k.logger.Info("sysmon stopped") + return nil + } + } +} +``` + +### Pattern 2: Deterministic ticker test via `testing/synctest` +**What:** Run the service inside a synctest "bubble" so the fake clock advances instantly and deterministically instead of sleeping in real time. +**When to use:** Any test exercising `time.Ticker`/`time.Sleep`-driven code — exactly `KeeperService.Run`. +**Example:** +```go +// internal/activity/service/keeper_service_test.go +package service_test + +import ( + "context" + "log/slog" + "sync" + "testing" + "testing/synctest" + "time" + + "sysmon/internal/activity/service" +) + +type fakePointer struct { + mu sync.Mutex + calls [][2]int +} + +func (f *fakePointer) Nudge(dx, dy int) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, [2]int{dx, dy}) + return nil +} + +func (f *fakePointer) callCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.calls) +} + +func TestKeeperService_NudgesOnEachTick(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + fake := &fakePointer{} + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(fake, logger, 10*time.Second, 1, 40*time.Millisecond) + + ctx, cancel := context.WithCancel(t.Context()) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + // Let 3 full ticks elapse -- instantly, on the bubble's fake clock. + time.Sleep(35 * time.Second) + synctest.Wait() // ensure all pending goroutine work (the +1/-1 nudge pairs) has settled + + if got := fake.callCount(); got != 6 { // 3 ticks * 2 calls (+1, -1) + t.Fatalf("callCount = %d, want 6", got) + } + + cancel() + synctest.Wait() + + if err := <-done; err != nil { + t.Fatalf("Run returned error: %v", err) + } + }) +} + +func TestKeeperService_StopsOnContextCancel(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + fake := &fakePointer{} + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(fake, logger, time.Second, 1, time.Millisecond) + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + synctest.Wait() // let Run reach its select{} and block + cancel() + + select { + case err := <-done: + if err != nil { + t.Fatalf("Run returned error: %v", err) + } + default: + synctest.Wait() // drain until Run's goroutine exits + <-done + } + }) +} +``` +**Source:** pattern verified against `go doc testing/synctest` output and go.dev/blog/testing-time — see Sources. + +### Anti-Patterns to Avoid +- **Sleeping in real time inside the test (`time.Sleep(3 * time.Second)` in a normal, non-synctest test):** Reintroduces flakiness under CI load and slows the suite. Avoid entirely now that `testing/synctest` is stable on this toolchain. +- **A package-level `var logger = slog.Default()` in `service`:** Breaks testability (LOG-01 explicitly requires injection) and prevents `-quiet` from being scoped per-instance. +- **Leaving a goroutine running past the end of a `synctest.Test` closure:** The bubble's root goroutine returning stops the fake clock; if a child goroutine is still blocked waiting for time to advance when the bubble ends, the test panics with a deadlock diagnostic. Always `cancel()` and `synctest.Wait()` before the closure returns (or before `<-done`). +- **Testing `CGPointer` directly:** Never construct or call the darwin adapter from tests — it requires Accessibility permission and moves the real cursor. Tests operate only on the `port.Pointer` interface via the fake. + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Deterministic time-based test | A custom fake ticker/clock abstraction (`type Clock interface{ Now() time.Time; Tick() <-chan time.Time }`) injected everywhere | `testing/synctest.Test` + real `time.Ticker` inside the bubble | Stdlib as of Go 1.25 (this repo runs 1.26.5); zero extra abstraction layers in production code, and it correctly fake-advances `time.Sleep` too (the 40ms nudge pause) without a second injection point. | +| Mock generation for `Pointer` | A mock-generation tool/library (gomock, testify/mock, moq) | A ~10-line hand-written `fakePointer` struct | One-method interface; locked decision is zero third-party deps; a generated mock adds a dependency and codegen step for no benefit here. | +| Signal → cancellation plumbing | Manual `signal.Notify(ch, ...)` + a `select` arm converting to context cancel | `signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)` | Stdlib since Go 1.16; directly returns a `context.Context` + `stop func()`, removing one manual translation layer from `main.go`. | + +**Key insight:** For a utility this size, the temptation is to add abstraction layers "for testability" (custom clock interfaces, DI containers). The stdlib as of Go 1.25/1.26 already solves the one hard problem (deterministic time) natively — reach for `testing/synctest` before reaching for a hand-rolled clock port. + +## Runtime State Inventory + +> Skipped — this is not a rename/refactor-of-identity phase (no renamed strings, IDs, or externally-registered names). It is a structural code reorganization of a stateless CLI. No databases, external services, OS-registered schedulers, secrets, or build artifacts reference the old flat-file layout by name. The Makefile's `go build -o $(BINARY) .` target is the one thing that must be updated to point at `./cmd/sysmon` — this is a code/build-config edit, not a runtime-state migration, and is called out explicitly under Common Pitfalls below. + +## Common Pitfalls + +### Pitfall 1: Makefile silently breaks after `main.go` moves +**What goes wrong:** `Makefile`'s `build` target runs `go build -o $(BINARY) .`, which builds the package in the current directory (repo root). After `main.go` moves to `cmd/sysmon/main.go` and the root `package main` file is deleted, `go build -o sysmon .` at the root will fail (no Go files) or silently build nothing useful. +**Why it happens:** BUILD-01 requires "Makefile targets keep working unchanged" — but the build target's *argument* must change even though its *interface* (`make build`) stays the same. +**How to avoid:** Update the `build` target to `go build -o $(BINARY) ./cmd/sysmon`. Verify with `make build && make run && make start && make status && make stop && make clean` end-to-end after the refactor — this satisfies BUILD-01 literally, not just structurally. +**Warning signs:** `make build` reports "no Go files in ..." or builds an empty/stale binary from `go.mod`'s module cache. + +### Pitfall 2: cgo adapter breaking `go test ./...` on non-darwin or in domain/service packages +**What goes wrong:** If the `//go:build darwin` tag is omitted or misplaced (e.g., only on the `.go` file but the package also has a non-tagged file, or the tag is a `// +build` old-style comment with a typo), `go vet`/`go test ./...` can fail to compile the adapter package on CI, or — worse — cgo directives leak into a file that `domain`/`service` accidentally imports transitively. +**Why it happens:** Build constraints are easy to get subtly wrong (blank line requirements, `//go:build` vs `// +build` syntax, package-level vs file-level placement). +**How to avoid:** Put `//go:build darwin` as the very first line of `cgpointer.go`, followed by a blank line, then `package adapter`. Confirm `domain` and `port` and `service` packages have zero cgo imports and zero references to `adapter` (import direction is inward only — `adapter` depends on `port`, never the reverse). Run `GOOS=linux go build ./internal/activity/domain/... ./internal/activity/port/... ./internal/activity/service/...` as a sanity check — it should succeed even though `adapter` (and thus the full `cmd/sysmon` binary) only builds on darwin. This is expected and acceptable per CONTEXT.md ("adapter package won't compile off-darwin — acceptable since CI is macOS"). +**Warning signs:** `go test ./...` from repo root fails with cgo/framework linker errors when run on a non-darwin CI runner or in a sandboxed environment without Xcode Command Line Tools — should only ever affect the `adapter` package, never `domain`/`port`/`service`. + +### Pitfall 3: `testing/synctest` bubble deadlock from a goroutine that never exits +**What goes wrong:** If the test spawns `go func() { svc.Run(ctx) }()` but never cancels `ctx` and never lets that goroutine return before the `synctest.Test` closure ends, Go panics with a bubble deadlock diagnostic (time cannot advance because the root goroutine returned while another goroutine in the bubble is still durably blocked). +**Why it happens:** `synctest.Test`'s fake clock stops advancing when the bubble's root goroutine exits; any child goroutine still waiting on that clock is now stuck forever. +**How to avoid:** Always pair the goroutine spawn with an explicit `cancel()` + drain of the result channel (`<-done`) before the `synctest.Test` closure returns, as shown in the Pattern 2 code sketch above. +**Warning signs:** Test panics with a message referencing "deadlock" or "goroutines are stuck" pointing at `testing/synctest` internals. + +### Pitfall 4: Forgetting `CFRelease` on both CGEventRefs when relocating the cgo block +**What goes wrong:** The current `main.go` calls `CFRelease` on both `cur` (from `CGEventCreate`) and `move` (from `CGEventCreateMouseEvent`) — two separate CoreFoundation objects per nudge. When relocating this C code into `cgpointer.go`, it's easy to accidentally drop one `CFRelease` (e.g., during a refactor that "cleans up" the C block) and introduce a slow CF-object leak that only shows up after hours of runtime (this is a long-running background utility per the project's core value). +**Why it happens:** The two `CFRelease` calls aren't adjacent to their allocations in a way that makes the pairing visually obvious once the code is reformatted or the function is split. +**How to avoid:** Per CONTEXT.md's Specific Ideas, keep the cgo C `nudge` helper "essentially as-is" — copy-paste the existing C block into `cgpointer.go` verbatim rather than rewriting it, changing only the surrounding Go glue (method receiver, error return). Diff the moved C code against `main.go`'s original block to confirm byte-for-byte equivalence of the CF lifecycle calls. +**Warning signs:** None observable in a short test run — this is the kind of bug that only appears after hours/days of production use (memory growth). Preserve the original C code exactly to avoid re-introducing a fixed issue. + +### Pitfall 5: `-quiet` implemented as output redirection instead of a level threshold +**What goes wrong:** The current `main.go` implements `-quiet` by doing `log.SetOutput(os.Stderr)` — redirecting rather than filtering. LOG-01 and CONTEXT.md explicitly require the *refactored* version to raise the level threshold (e.g., warn+) instead. If the refactor naively ports the old redirect-based logic into an `slog.Handler` wrapper, it technically "logs less to stdout" but violates the locked design (structured level-based filtering) and makes the two lifecycle log lines (start/stop) — which are Info-level — silently disappear or reappear inconsistently depending on which stream a test asserts against. +**Why it happens:** Redirection and level-filtering both "look like" they satisfy "-quiet suppresses logs to stdout" from a black-box behavior view, but only one matches the locked slog-based design. +**How to avoid:** In `main.go`, choose `slog.LevelWarn` (or higher) as the handler's minimum level when `-quiet` is set, `slog.LevelInfo` otherwise — do not touch the output stream at all. Example: +```go +level := slog.LevelInfo +if *quiet { + level = slog.LevelWarn +} +handler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level}) +logger := slog.New(handler) +``` +Since the current lifecycle logs (start, stop) are informational, under `-quiet` they simply won't meet the Warn threshold and won't print — matching the observed behavior ("no logs to stdout when quiet") through a different, more idiomatic mechanism. +**Warning signs:** A test or manual check expecting zero stdout output under `-quiet` still sees output because the handler's level wasn't actually raised (e.g., `HandlerOptions.Level` left `nil`, which defaults to Info). + +## Code Examples + +### Domain value objects (Interval, Offset) +```go +// internal/activity/domain/activity.go +package domain + +import ( + "errors" + "time" +) + +// Interval is a validated positive duration between nudges. +type Interval struct { + d time.Duration +} + +func NewInterval(d time.Duration) (Interval, error) { + if d <= 0 { + return Interval{}, errors.New("interval must be greater than zero") + } + return Interval{d: d}, nil +} + +func (i Interval) Duration() time.Duration { return i.d } + +// Offset is the pixel shift magnitude applied then reversed on each nudge. +type Offset int + +func NewOffset(px int) (Offset, error) { + if px <= 0 { + return 0, errors.New("offset must be greater than zero") + } + return Offset(px), nil +} +``` +Note: per CONTEXT.md "Claude's Discretion," the exact shape (whether `Offset` is a distinct type or a plain constant) is left open — the above is one clean option; a bare `const defaultOffset = 1` in `service` is equally acceptable if the team prefers less ceremony for a single hard-coded value that TASK.md never asks to be user-configurable. + +### Ports +```go +// internal/activity/port/pointer.go +package port + +// Pointer abstracts moving the system pointer by a relative offset and +// posting a synthetic mouse-moved event, which resets the OS idle timer. +type Pointer interface { + Nudge(dx, dy int) error +} +``` +```go +// internal/activity/port/keeper.go +package port + +import "context" + +// Keeper runs the activity-keeping loop until ctx is done. +type Keeper interface { + Run(ctx context.Context) error +} +``` + +### cgo adapter (relocated, essentially unchanged) +```go +//go:build darwin + +// internal/activity/adapter/cgpointer.go +package adapter + +/* +#cgo LDFLAGS: -framework ApplicationServices -framework CoreGraphics +#include + +static void nudge(int dx, int dy) { + CGEventRef cur = CGEventCreate(NULL); + CGPoint p = CGEventGetLocation(cur); + CFRelease(cur); + + CGPoint np = CGPointMake(p.x + dx, p.y + dy); + CGEventRef move = CGEventCreateMouseEvent(NULL, kCGEventMouseMoved, np, kCGMouseButtonLeft); + CGEventPost(kCGHIDEventTap, move); + CFRelease(move); +} +*/ +import "C" + +// CGPointer implements port.Pointer using CoreGraphics on macOS. +type CGPointer struct{} + +func NewCGPointer() *CGPointer { return &CGPointer{} } + +func (c *CGPointer) Nudge(dx, dy int) error { + C.nudge(C.int(dx), C.int(dy)) + return nil +} +``` + +### `cmd/sysmon/main.go` wiring +```go +package main + +import ( + "context" + "flag" + "log/slog" + "os" + "syscall" + "time" + + "sysmon/internal/activity/adapter" + "sysmon/internal/activity/service" +) + +const ( + defaultInterval = 10 * time.Second + nudgeOffset = 1 + nudgePause = 40 * time.Millisecond +) + +func main() { + interval := flag.Duration("interval", defaultInterval, "how often to nudge the cursor (e.g. 10s, 30s, 1m)") + quiet := flag.Bool("quiet", false, "do not write logs to stdout") + flag.Parse() + + level := slog.LevelInfo + if *quiet { + level = slog.LevelWarn + } + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level})) + + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + pointer := adapter.NewCGPointer() + keeper := service.NewKeeperService(pointer, logger, *interval, nudgeOffset, nudgePause) + + if err := keeper.Run(ctx); err != nil { + logger.Error("sysmon exited with error", "err", err) + os.Exit(1) + } +} +``` +(Import `"os/signal"` alongside the others above — omitted from the snippet's import block only for brevity; include it in the actual file.) + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|---------------|--------| +| `signal.Notify(ch, sig...)` + manual `select` arm to detect shutdown | `signal.NotifyContext(ctx, sig...)` returning a ready-made `context.Context` | Go 1.16 (2021) | Collapses signal handling directly into the context-cancellation idiom the rest of the app already uses for `Keeper.Run(ctx)`. | +| Hand-rolled fake clock/ticker interfaces injected for testability | `testing/synctest` fake-clock "bubble" | Go 1.24 experimental (2024) -> **stable in Go 1.25** (Aug 2025); `Run` renamed to `Test` at stabilization | Removes the need for a custom `Clock`/`Ticker` port purely for test determinism; real `time.Ticker`/`time.Sleep` "just work" deterministically inside `synctest.Test`. This repo's `go 1.26.5` toolchain has it natively. | +| `log.Printf`/`log.Println` with `log.SetOutput` for quiet mode | `log/slog` with `HandlerOptions.Level` threshold, injected `*slog.Logger` | slog added Go 1.21 (2023); this is a project-local behavior change (LOG-01) | Structured, leveled logs; `-quiet` becomes a level filter rather than a stream redirect; testable without touching global output. | + +**Deprecated/outdated:** +- `testing/synctest.Run` (the pre-stabilization experimental name): deprecated in favor of `Test` as of Go 1.25; do not use `Run` in new code — verified via `go doc testing/synctest`, which shows only `Test` and `Wait` as exported functions on this toolchain. +- `GOEXPERIMENT=synctest`: no longer required; the package is a normal stdlib import on Go 1.25+ (this repo is on 1.26.5). + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| A1 | `HandlerOptions.Level` set to `slog.LevelWarn` under `-quiet` is an acceptable interpretation of "raise the level threshold" from CONTEXT.md, and that lifecycle Info logs (start/stop) are the only logs currently emitted | Common Pitfalls (Pitfall 5), Code Examples | Low — if future log statements are added at Info level that must still show under `-quiet`, the threshold choice may need revisiting; easy to adjust (single constant). | +| A2 | `Offset` as a validated domain type (vs. a bare constant) is the preferred shape | Code Examples (domain) | Low — CONTEXT.md explicitly leaves this to Claude's discretion; either shape satisfies ARCH-04. | + +**All other claims in this research were verified directly against the installed Go 1.26.5 toolchain (`go doc`) or cross-checked against go.dev official documentation/blog posts** — no user confirmation needed for the `testing/synctest` recommendation, the `slog.DiscardHandler` mechanism, or `signal.NotifyContext`'s availability. + +## Open Questions + +1. **Should `KeeperService.Run` accept interval/offset/pause via constructor (as sketched above) or via `Run` method parameters?** + - What we know: CONTEXT.md explicitly defers this to Claude's discretion ("Whether `Keeper.Run` takes the interval/offset via constructor or method"). + - What's unclear: No strong technical reason to prefer one over the other for a single-shot CLI process. + - Recommendation: Constructor injection (as shown) — `Run(ctx)` matches the `port.Keeper` interface signature exactly (`Run(ctx) error`), keeping the port interface simple and making `main.go`'s wiring linear (build once, run once). This also matches the pattern already used for `Pointer` and `logger` injection, for consistency. + +2. **Does `nudgePause` (40ms) belong in `domain` as a validated type, or as a plain `time.Duration` constant in `service`/`cmd`?** + - What we know: TASK.md and CONTEXT.md only call out `Interval` and `Offset` explicitly as domain candidates; the pause is described as "hard-coded for reproducibility" in the existing CLAUDE.md conventions notes. + - What's unclear: Whether promoting it to a domain type adds meaningful validation value (it's never user-configurable, unlike interval). + - Recommendation: Keep `nudgePause` as a plain constant (in `cmd/sysmon/main.go`, passed through to the service constructor) — do not over-engineer a value that has no validation rule and no user-facing flag, consistent with CONTEXT.md's "keep it small; do not over-engineer" guidance. + +## Environment Availability + +| Dependency | Required By | Available | Version | Fallback | +|------------|------------|-----------|---------|----------| +| Go toolchain | Entire build/test | Yes | go1.26.5 darwin/arm64 (confirmed via `go version`) | — | +| `testing/synctest` (stdlib) | Deterministic ticker test (TEST-01) | Yes | Stable since go1.25; present in go1.26.5 (confirmed via `go doc testing/synctest`) | If ever downgraded below go1.25, fall back to an injected `<-chan time.Time` tick source in `KeeperService` | +| Xcode Command Line Tools / CoreGraphics frameworks | `adapter.CGPointer` build (cgo) | Assumed present (per CLAUDE.md platform requirements; existing `main.go` already builds with cgo on this machine) | — | None needed — `domain`/`port`/`service` compile and test without cgo; only the darwin adapter and full binary require it | +| macOS Accessibility permission | Runtime delivery of the real mouse event (not build/test) | Out of scope for this phase's automated tests — TEST-01 explicitly requires the test to run without it | — | N/A (manual verification only, already tracked in TASK.md §8 acceptance criteria) | + +**Missing dependencies with no fallback:** None identified. + +**Missing dependencies with fallback:** None currently missing; the `testing/synctest` fallback above is documented preemptively in case of a future Go downgrade. + +## Validation Architecture + +### Test Framework +| Property | Value | +|----------|-------| +| Framework | Go stdlib `testing` + `testing/synctest` (stable, go1.25+) | +| Config file | none — no test framework config needed for stdlib `go test` | +| Quick run command | `go test ./internal/... -run TestKeeperService -v` | +| Full suite command | `go test -race ./...` (required by CONTEXT.md; note this only exercises `domain`/`port`/`service` meaningfully in CI unless run on a darwin runner with cgo enabled for the `adapter` package and `cmd/sysmon`) | + +### Phase Requirements -> Test Map +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| TEST-01 | `KeeperService` nudges on each tick via fake `Pointer`, no real mouse | unit | `go test ./internal/activity/service/... -run TestKeeperService_NudgesOnEachTick -race` | Wave 0 | +| TEST-01 | `KeeperService.Run` stops cleanly on context cancellation | unit | `go test ./internal/activity/service/... -run TestKeeperService_StopsOnContextCancel -race` | Wave 0 | +| ARCH-04 | `domain` package has no cgo/os/system imports | build check | `GOOS=linux GOARCH=amd64 go build ./internal/activity/domain/... ./internal/activity/port/... ./internal/activity/service/...` (should succeed without cgo/darwin) | Wave 0 | +| BUILD-01 | All Makefile targets still work post-refactor | manual/smoke | `make build && make run` (Ctrl+C) `&& make start && make status && make stop && make clean` | Wave 0 (Makefile edit) | +| BUILD-02 | `-interval`/`-quiet` flags, default 10s, clean SIGINT/SIGTERM exit code 0 | manual smoke (already Validated per PROJECT.md for existing behavior; re-verify post-move) | `./sysmon -interval 300ms` then `kill -TERM `; `echo $?` | Wave 0 | +| LOG-01 | `-quiet` raises level threshold; lifecycle events logged with structure | unit (handler level assertion) or manual | `go test ./cmd/... -run TestQuietRaisesLevel` (optional) or manual `./sysmon -quiet` stdout check | Wave 0 (optional unit test) | + +### Sampling Rate +- **Per task commit:** `go test ./internal/activity/... -race` +- **Per wave merge:** `go test -race ./...` (on darwin, cgo-enabled — this is the only environment where the full module including `adapter`/`cmd` compiles) +- **Phase gate:** Full suite green (`go test -race ./...` on darwin) before `/gsd-verify-work`, plus the manual Makefile smoke sequence above (BUILD-01/BUILD-02 are behavior-preservation requirements that automated tests can only partially cover — e.g., "real mouse doesn't visibly drift" and "screen doesn't sleep" remain manual per TASK.md §8). + +### Wave 0 Gaps +- [ ] `internal/activity/service/keeper_service_test.go` — covers TEST-01 (both sub-behaviors: nudge-per-tick, stop-on-cancel) +- [ ] `Makefile` edit — `build` target must point at `./cmd/sysmon` instead of `.` (see Pitfall 1) — required for BUILD-01 +- [ ] No new test framework install needed — `testing`/`testing/synctest` are stdlib, already available on go1.26.5 + +## Security Domain + +### Applicable ASVS Categories + +| ASVS Category | Applies | Standard Control | +|---------------|---------|-------------------| +| V2 Authentication | No | N/A — single-user local CLI, no auth boundary | +| V3 Session Management | No | N/A | +| V4 Access Control | No | N/A — relies on macOS TCC (Accessibility permission), an OS-level control already documented in TASK.md §7, unchanged by this refactor | +| V5 Input Validation | Yes (minimal) | `domain.NewInterval` rejects `<= 0` durations; `flag.Duration` already rejects unparseable duration strings at the `flag` package level before reaching domain code | +| V6 Cryptography | No | N/A — no cryptographic operations in this utility | + +### Known Threat Patterns for this stack + +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|----------------------| +| Unreleased CoreFoundation objects (`CGEventRef`) causing unbounded memory growth in a long-running background process | Denial of Service (resource exhaustion) | Preserve both `CFRelease` calls exactly as in the original `nudge` C function (see Pitfall 4) — this is the only "security-adjacent" concern in an otherwise trust-boundary-free local utility. | +| Negative/zero interval causing a busy-loop (`time.NewTicker` panics on `d <= 0`) | Denial of Service (local resource exhaustion / crash) | `domain.NewInterval` validation rejects non-positive durations before a `time.Ticker` is ever constructed with them (ARCH-04's stated purpose). | + +This phase has no network exposure, no authentication boundary, and no data persistence — the security surface is limited to the two items above, both already addressed by the locked design (domain validation + CFRelease preservation). + +## Sources + +### Primary (HIGH confidence — verified directly against installed toolchain) +- `go doc testing/synctest` (run locally against go1.26.5) — confirmed `Test`/`Wait` signatures, fake-clock semantics, deadlock-on-early-return behavior +- `go doc log/slog.DiscardHandler`, `go doc log/slog.HandlerOptions` (run locally) — confirmed `DiscardHandler` and `HandlerOptions.Level` mechanics +- `go doc os/signal.NotifyContext` (run locally) — confirmed signature and cancellation semantics +- `go version` (run locally) — confirmed `go1.26.5 darwin/arm64`, satisfying `testing/synctest`'s go1.25 minimum + +### Secondary (MEDIUM confidence — official docs/blog, cross-checked) +- https://pkg.go.dev/testing/synctest — package doc, `Test`/`Wait` API, fake-clock behavior +- https://go.dev/blog/synctest — introduction of the experimental `Run`-based API in Go 1.24 (superseded) +- https://go.dev/blog/testing-time — stabilization details, `Run`→`Test` rename, root-goroutine-exit-stops-clock behavior +- https://go.dev/doc/go1.25 — Go 1.25 release notes confirming synctest graduated from experiment to GA + +### Tertiary (LOW confidence — community sources, used only for corroboration) +- https://boyter.org/posts/golang-slog-disable-tests/ — `slog.SetDefault(slog.New(slog.DiscardHandler))` pattern for test suites (corroborated by local `go doc` output, not relied on alone) + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — 100% stdlib, versions confirmed against the actual installed toolchain via `go doc`/`go version`, not memory +- Architecture: HIGH — directly specified by `docs/TASK.md` §10 and `01-CONTEXT.md`; no open architectural questions +- Pitfalls: HIGH for cgo/CFRelease/Makefile items (grounded in reading the actual `main.go` and `Makefile`); MEDIUM for the synctest deadlock pitfall (grounded in official docs, not yet reproduced in this repo's own test suite) + +**Research date:** 2026-07-12 +**Valid until:** 2026-10-10 (approx. 90 days — stdlib-only research on a pinned Go version is unusually stable; revisit only if `go.mod`'s Go version changes) + diff --git a/.planning/phases/01-hexagonal-refactor/01-REVIEW-2.md b/.planning/phases/01-hexagonal-refactor/01-REVIEW-2.md new file mode 100644 index 0000000..1c2116c --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-REVIEW-2.md @@ -0,0 +1,145 @@ +--- +phase: 01-hexagonal-refactor +reviewed: 2026-07-12T00:00:00Z +depth: standard +files_reviewed: 10 +files_reviewed_list: + - cmd/sysmon/main.go + - internal/activity/domain/activity.go + - internal/activity/domain/activity_test.go + - internal/activity/port/pointer.go + - internal/activity/port/keeper.go + - internal/activity/service/keeper_service.go + - internal/activity/service/keeper_service_test.go + - internal/activity/adapter/cgpointer.go + - Makefile + - .golangci.yml +findings: + critical: 0 + warning: 1 + info: 1 + total: 2 +status: issues_found +--- + +# Phase 1: Code Review Report (second pass, post-fix) + +**Reviewed:** 2026-07-12 +**Depth:** standard +**Files Reviewed:** 10 +**Status:** issues_found + +## Summary + +All three findings from review #1 are genuinely and correctly resolved, with +no regressions introduced. + +- **CR-01 (domain validation never wired)** — fixed. `run()` now calls + `domain.NewInterval(*interval)` and `domain.NewOffset(nudgeOffset)` + (`main.go:56-65`) and exits with code 2 and a clear `slog` error on failure + instead of reaching `time.NewTicker`. Verified by reproduction: + `./sysmon -interval=-5s` and `-interval=0s` both now print + `level=ERROR msg="invalid interval" err="interval must be greater than zero"` + and exit 2, no panic. `KeeperService` was retyped to accept + `domain.Interval`/`domain.Offset` directly (`keeper_service.go:35`), so the + raw-primitive bypass this finding described is closed at the type level, not + just at the one call site in `main.go`. +- **WR-02 (positional-argument-order footgun)** — fixed as a side effect of the + above. `NewKeeperService(pointer port.Pointer, logger *slog.Logger, interval + domain.Interval, offset domain.Offset)` no longer has two adjacent + `time.Duration`-typed parameters; `nudgePause` was demoted from a + constructor parameter to an unexported package constant + (`keeper_service.go:18`), so there is nothing left to swap by accident. +- **WR-01 (uninterruptible `time.Sleep(pause)` delaying shutdown)** — fixed + correctly. The pause is now `select { case <-time.After(nudgePause): + case <-ctx.Done(): }` inside the new `nudgeCycle` helper + (`keeper_service.go:84-87`), and `Run`'s ticker branch checks `ctx.Err()` + immediately after `nudgeCycle` returns and exits promptly + (`keeper_service.go:58-65`) rather than looping back to `select` and waiting + for the next tick. Confirmed no double "sysmon stopped" log is possible: + once the ticker branch returns, the `ctx.Done()` branch is never reached. +- Layering, cgo, and build hygiene are all still clean: `domain` has zero + port/service/adapter imports, `service` doesn't import `adapter` (verified + via `go list -deps`), both `CFRelease` calls are intact in `cgpointer.go`, + `//go:build darwin` is present, and `go build`, `go vet`, + `golangci-lint run ./...` (0 issues), and `go test -race ./...` are all + clean. + +One design choice in the WR-01 fix introduces a new, narrower gap that wasn't +present in review #1's assessment (since the interruptible pause didn't exist +yet): `nudgeCycle` still fires the compensating back-`Nudge` even when +`ctx.Done()` is what woke it from the pause, and this specific interleaving +(cancel arriving strictly between the forward and backward nudge) is not +exercised by either test. It's a reasonable behavior choice (matches the +pre-refactor original's "always complete the pair" semantics more closely +than an early return would), but it's undocumented and untested, so it's +filed as a Warning rather than accepted silently. + +## Warnings + +### WR-01: New interruptible-pause branch (nudgeCycle mid-pause cancellation) is untested + +**File:** `internal/activity/service/keeper_service.go:83-91` +**Issue:** `nudgeCycle`'s pause is now raced against `ctx.Done()`: +```go +select { +case <-time.After(nudgePause): +case <-ctx.Done(): +} +if err := k.pointer.Nudge(-offset, 0); err != nil { + k.logger.Error("nudge back failed", "err", err) +} +``` +Regardless of which branch fires, the backward `Nudge` still executes +unconditionally afterward. This is a defensible choice (it completes the +there-and-back pair rather than leaving the cursor 1px offset, matching how +the pre-refactor `main.go` always completed both `C.nudge` calls before ever +checking the signal channel), but neither existing test exercises this +interleaving: +- `TestKeeperService_StopsOnContextCancel` cancels before any tick ever + fires (1s interval, cancelled immediately after the first `synctest.Wait()`), + so it only exercises `Run`'s outer `<-ctx.Done()` select branch, never + `nudgeCycle`'s internal one. +- `TestKeeperService_NudgesPerTick` cancels only after all 3 expected ticks' + nudge pairs have fully completed (`time.Sleep(ticks*interval)` then + `synctest.Wait()` before `cancel()`), so the pause's `ctx.Done()` arm is + never selected either. + +As a result, the exact code path review #1's WR-01 asked to add (interruptible +pause) is present but has zero test coverage of its cancellation arm, and the +doc comment ("the pause between the two nudges is interruptible so shutdown is +not delayed by up to nudgePause", `keeper_service.go:74`) slightly overstates +promptness: shutdown latency is still bounded by however long the second +`Nudge` call takes to return (unbounded in principle for a future `Pointer` +implementation, though negligible for `CGPointer`), since it's called +unconditionally after the race resolves either way. +**Fix:** Add a synctest case that cancels ctx strictly inside the 40ms pause +window (e.g. schedule a tick, let `nudgeCycle` start, `time.Sleep(20*time. +Millisecond)` then `cancel()` before the pause's `time.After(nudgePause)` +fires) and assert the backward `Nudge` still occurs and `Run` returns +promptly. If the "always complete the pair" behavior is intentional, say so +explicitly in the doc comment on `nudgeCycle` rather than implying early +return; if early return (skip the back-nudge on cancel) is actually preferred, +change the code to `return` in the `ctx.Done()` case instead. + +## Info + +### IN-01: `nudgeCycle` doc comment overstates what "interruptible" guarantees + +**File:** `internal/activity/service/keeper_service.go:73-74` +**Issue:** The comment "The pause between the two nudges is interruptible so +shutdown is not delayed by up to nudgePause" is true only for the pause +itself; it does not mention that the backward `Nudge` call still runs after +an interrupted pause, which is the behavior that actually determines total +shutdown latency in that branch. A future reader (or the next reviewer) could +reasonably assume cancellation short-circuits the entire cycle. +**Fix:** Tighten the comment, e.g.: "The pause between the two nudges races +against ctx cancellation so it doesn't block for the full nudgePause on +shutdown; the compensating back-nudge still runs afterward either way, to +avoid leaving the cursor offset." + +--- + +_Reviewed: 2026-07-12_ +_Reviewer: Claude (gsd-code-reviewer)_ +_Depth: standard_ diff --git a/.planning/phases/01-hexagonal-refactor/01-REVIEW.md b/.planning/phases/01-hexagonal-refactor/01-REVIEW.md new file mode 100644 index 0000000..e355790 --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-REVIEW.md @@ -0,0 +1,212 @@ +--- +phase: 01-hexagonal-refactor +reviewed: 2026-07-12T00:00:00Z +depth: standard +files_reviewed: 10 +files_reviewed_list: + - cmd/sysmon/main.go + - internal/activity/domain/activity.go + - internal/activity/domain/activity_test.go + - internal/activity/port/pointer.go + - internal/activity/port/keeper.go + - internal/activity/service/keeper_service.go + - internal/activity/service/keeper_service_test.go + - internal/activity/adapter/cgpointer.go + - Makefile + - .golangci.yml +findings: + critical: 1 + warning: 2 + info: 2 + total: 5 +status: issues_found +--- + +# Phase 1: Code Review Report + +**Reviewed:** 2026-07-12 +**Depth:** standard +**Files Reviewed:** 10 +**Status:** issues_found + +## Summary + +The hexagonal split itself is clean: `domain` has zero cgo/os/port/service/adapter +imports and builds under `GOOS=linux` (verified), `service` depends only on +`domain`... except it doesn't, see CR-01 below... and `port`, never `adapter` +(verified via `go list -deps`), `adapter` is the sole cgo package, correctly +tagged `//go:build darwin`, and both `CFRelease` calls are preserved verbatim +from the original `main.go`. `go vet`, `go build ./...`, and `golangci-lint run +./...` are all clean, and `go test -race ./...` passes. + +However, tracing the actual data flow from `cmd/sysmon/main.go` into +`service.NewKeeperService` turns up one real, provable regression: the new +`domain.Interval`/`domain.Offset` validation is never invoked anywhere outside +its own test file. `main.go` passes the raw `*flag.Duration` and an untyped +`int` constant straight into the service, bypassing the domain package +entirely. This isn't just dead code (Info-tier) — because `flag.Duration` +accepts negative values and `time.NewTicker` panics on non-positive durations, +a user running `sysmon -interval=-5s` (or `-interval=0s`) now crashes with an +unhandled panic, which the pre-refactor `main.go` also would have crashed on, +but the entire point of adding `domain.Interval` in this phase was to prevent +exactly that class of bug ("preventing a busy-loop or a panic from a +non-positive time.Ticker" per its own doc comment). The validation was built +but never wired in, so the phase's own stated goal (ARCH-04) is unmet in +practice despite the unit tests for `domain` passing in isolation. + +Two lower-severity findings: the ticker loop's `time.Sleep(k.pause)` between +the two nudges is not selected against `ctx.Done()`, so shutdown latency can +be delayed by up to `pause` (bounded, small, but avoidable); and the second +`Nudge` (the "nudge back") on tick has no compensating action if it fails, +same as before, which is fine but the resulting drift is now unbounded +un-nudged-back state that's harder to reason about than the pre-refactor +inline code. + +## Critical Issues + +### CR-01: domain.Interval/domain.Offset validation exists but is never called from production code — negative/zero interval panics + +**File:** `cmd/sysmon/main.go:37,51` +**Issue:** `flag.Duration("interval", ...)` accepts any `time.Duration`, including +zero and negative values, with no validation. `run()` passes `*interval` +directly to `service.NewKeeperService` (`keeper_service.go:28`), which stores +it as a raw `time.Duration` and later calls `time.NewTicker(k.interval)` +(`keeper_service.go:44`). `time.NewTicker` panics if its argument is `<= 0`. +`domain.NewInterval` was built in this phase specifically to reject `d <= 0` +(`activity.go:28-33`, doc comment: "preventing a busy-loop or a panic from a +non-positive time.Ticker"), but neither `main.go` nor +`service.KeeperService` ever calls `domain.NewInterval` or +`domain.NewOffset` — confirmed by `grep -rl "internal/activity/domain" +--include="*.go" | grep -v _test.go` returning zero non-test files, and by +reproduction: `./sysmon -interval=-5s` panics with +`panic: non-positive interval for NewTicker`. +This is a regression versus even the flat pre-refactor `main.go` in spirit: +the phase explicitly added this guard rail (ARCH-04, "Interval that validates +> 0") and it is inert. `KeeperService.interval`/`offset` are plain +`time.Duration`/`int` fields, not `domain.Interval`/`domain.Offset`, so there +is no type-level enforcement either. +**Fix:** Have `run()` construct a `domain.Interval` from the flag value and +handle the error before constructing the service (print a usage error and +`return 1` rather than panicking), and change `KeeperService`'s constructor +signature to accept `domain.Interval`/`domain.Offset` instead of raw +primitives so this can't regress silently again: +```go +// cmd/sysmon/main.go +iv, err := domain.NewInterval(*interval) +if err != nil { + logger.Error("invalid -interval", "err", err) + return 1 +} +off, err := domain.NewOffset(nudgeOffset) +if err != nil { + logger.Error("invalid nudge offset", "err", err) + return 1 +} +keeper := service.NewKeeperService(pointer, logger, iv, off, nudgePause) +``` +```go +// internal/activity/service/keeper_service.go +func NewKeeperService(pointer port.Pointer, logger *slog.Logger, interval domain.Interval, offset domain.Offset, pause time.Duration) *KeeperService { + return &KeeperService{ + pointer: pointer, + logger: logger, + interval: interval.Duration(), + offset: offset.Int(), + pause: pause, + } +} +``` +Update `keeper_service_test.go` call sites accordingly (they currently pass +raw `int`/`time.Duration` literals too, so they'd need `domain.NewInterval`/ +`domain.NewOffset` helpers or exported test constructors). + +## Warnings + +### WR-01: Shutdown can be delayed up to `pause` because `time.Sleep` isn't selected against `ctx.Done()` + +**File:** `internal/activity/service/keeper_service.go:56` +**Issue:** Inside the `case <-ticker.C:` branch, after the first `Nudge` +succeeds, the code calls `time.Sleep(k.pause)` unconditionally before issuing +the second `Nudge`. If `ctx` is cancelled during that sleep (e.g. SIGTERM +arrives 1ms into a 40ms pause), the cancellation is not observed until the +sleep finishes and the loop re-enters `select`. This is bounded by `pause` +(40ms default) so it's not severe, but it's a real, avoidable deviation from +"stops cleanly on ctx cancellation" as stated in the package doc comment +(`keeper_service.go:15-16`), and it silently degrades the "clean SIGINT/SIGTERM +exit" behavior preserved from the original by up to `pause` on every run that +happens to be cancelled mid-nudge. +**Fix:** Race the sleep against `ctx.Done()`: +```go +select { +case <-time.After(k.pause): +case <-ctx.Done(): + k.logger.Info("sysmon stopped") + return nil +} +``` +Given `pause` is only 40ms, this is low-impact and could reasonably be +accepted as-is, but it should be a conscious tradeoff, not an oversight. + +### WR-02: KeeperService constructor takes 5 positional same-typed-adjacent params, inviting argument-order mistakes + +**File:** `internal/activity/service/keeper_service.go:28` +**Issue:** `NewKeeperService(pointer port.Pointer, logger *slog.Logger, +interval time.Duration, offset int, pause time.Duration)` has two +`time.Duration` parameters (`interval`, `pause`) separated only by an `int`. +Nothing at the call site or type system prevents swapping `interval` and +`pause` (both compile fine either order) — e.g. +`NewKeeperService(pointer, logger, pause, offset, interval)` would silently +tick every 40ms and pause for 10s between nudges. This is exactly the class of +bug the locked design's domain types (`Interval`) were meant to close off +(see CR-01) — once `domain.Interval` is actually threaded through as +recommended in CR-01's fix, this concern is resolved as a side effect since +`domain.Interval` and a raw `pause time.Duration` are no longer +interchangeable by accident... but `pause` itself is still a bare +`time.Duration` type-identical to nothing else, so this is a narrower residual +concern once CR-01 lands. Filed separately because it stands on its own even +if CR-01's fix is deferred. +**Fix:** Adopt CR-01's fix (typed `domain.Interval`/`domain.Offset` +parameters), which also makes `interval`/`pause` non-interchangeable at +the type level since only `interval` becomes `domain.Interval`. If `pause` +stays a raw `time.Duration`, consider a small named type or a functional-options +constructor to remove positional ambiguity entirely. + +## Info + +### IN-01: `nudgeOffset` and `nudgePause` are untyped/unvalidated constants duplicated between `main.go` and tests + +**File:** `cmd/sysmon/main.go:24,26` +**Issue:** `nudgeOffset = 1` and `nudgePause = 40 * time.Millisecond` are +plain package-level consts in `cmd/sysmon`, matching CONTEXT.md's explicit +discretion note ("no domain validation needed"). This is acceptable per the +locked design, but worth flagging alongside CR-01: since `domain.Offset` +exists and is unused, there's now redundant modeling (a validated `Offset` +type nobody constructs, and a bare `int` constant that's actually used) that +reads as unfinished rather than intentional. Once CR-01's fix is applied this +tension disappears since `nudgeOffset` would flow through `domain.NewOffset`. +**Fix:** No standalone action needed if CR-01 is fixed; otherwise, remove +`domain.Offset`/`NewOffset` to avoid presenting unused validation as if it +were live. + +### IN-02: `KeeperService.Run` continues the loop after a failed forward `Nudge`, silently skipping the compensating back-nudge + +**File:** `internal/activity/service/keeper_service.go:52-55` +**Issue:** If the first `Nudge(k.offset, 0)` returns an error, the code logs +and `continue`s — correctly skipping `time.Sleep` and the second `Nudge` since +there's nothing to compensate for. This matches the original's implicit +behavior (the original never checked errors at all) and is not a bug, but the +asymmetry is worth documenting: if the forward nudge partially succeeds at the +OS level but still returns an error (unlikely with the current `CGPointer`, +which never returns non-nil, but possible for a future adapter), the cursor +could drift by `offset` px per such event with no recovery path. Not +actionable now since `CGPointer.Nudge` always returns `nil` (`cgpointer.go:38-41`), +but flagging for future adapters. +**Fix:** No change required today. If a future `Pointer` implementation can +fail after partially moving the cursor, consider always attempting the +compensating `Nudge(-k.offset, 0)` even when the forward call errors. + +--- + +_Reviewed: 2026-07-12_ +_Reviewer: Claude (gsd-code-reviewer)_ +_Depth: standard_ diff --git a/.planning/phases/01-hexagonal-refactor/01-VALIDATION.md b/.planning/phases/01-hexagonal-refactor/01-VALIDATION.md new file mode 100644 index 0000000..b21842b --- /dev/null +++ b/.planning/phases/01-hexagonal-refactor/01-VALIDATION.md @@ -0,0 +1,23 @@ +# Phase 1 — Validation Strategy + +How each requirement is proven. Automated checks run in CI (build/lint/test on +macOS); a couple need a one-time manual check on the target Mac. + +| REQ | Validation method | Type | +|-----|-------------------|------| +| ARCH-01 | Repo contains `cmd/sysmon/main.go` + `internal/activity/{domain,port,service,adapter}`; no root `main.go`. Checked by `go build ./...` + directory assertion. | build/structural | +| ARCH-02 | `Pointer` interface in `port`; `CGPointer` in `adapter` with `//go:build darwin`; `grep` confirms build tag + `CFRelease` present. | structural | +| ARCH-03 | `KeeperService` in `service` implements `Keeper`; imports show only `domain`+`port` (no `adapter`). Verified by import inspection / `go list -deps`. | structural | +| ARCH-04 | `domain` imports nothing from `port`/`service`/`adapter` and no cgo/os; `GOOS=linux go build ./internal/activity/dom/...` compiles (no cgo). | structural | +| ARCH-05 | No `main.go` at repo root (`test ! -f main.go`). | structural | +| TEST-01 | `go test -race ./...` passes a `KeeperService` unit test using a **mockgen-generated** `Pointer` mock (`go.uber.org/mock`), **testify** assertions, and `testing/synctest` for deterministic time; no real mouse, no Accessibility needed. | automated test | +| LOG-01 | `KeeperService` takes an injected `*slog.Logger`; no package-global logger (grep); `-quiet` raises level to warn. Unit test asserts via a capturing slog handler + testify. | automated test | +| BUILD-01 | `go build` succeeds; all Makefile targets work; `make build` builds `./cmd/sysmon`. | build/manual | +| BUILD-02 | Behavior preserved: `-interval`/`-quiet` flags, 10s default, real `kCGEventMouseMoved`, clean SIGINT/SIGTERM (exit 0). Flags/exit covered by test/manual; event delivery = manual on Mac. | automated + manual | + +## Manual checks (one-time, on the target Mac, after merge) +- Grant Accessibility; run `make start`; confirm the screen does not sleep; `make stop` kills only sysmon. (BUILD-02 event delivery, from docs/TASK.md §8.) + +## Notes +- Domain/service/port must be buildable and testable WITHOUT cgo/macOS so the + test suite is portable and fast; only the `adapter` package requires darwin. diff --git a/Makefile b/Makefile index a4c6112..1132301 100644 --- a/Makefile +++ b/Makefile @@ -1,20 +1,23 @@ BINARY := sysmon PIDFILE := /tmp/$(BINARY).pid +# INTERVAL controls how often the cursor is nudged. Override on the command +# line, e.g. `make start INTERVAL=3s`. Must be a positive Go duration. +INTERVAL ?= 10s .PHONY: build run start stop status clean ## build: build the binary build: - go build -o $(BINARY) . + go build -o $(BINARY) ./cmd/sysmon ## run: build and run in the current terminal (Ctrl+C to quit) run: build - ./$(BINARY) + ./$(BINARY) -interval $(INTERVAL) ## start: build and run in the background with no window start: build - @nohup ./$(BINARY) -quiet >/dev/null 2>&1 & echo $$! > $(PIDFILE) - @echo "sysmon started in the background (PID $$(cat $(PIDFILE)))" + @nohup ./$(BINARY) -quiet -interval $(INTERVAL) >/dev/null 2>&1 & echo $$! > $(PIDFILE) + @echo "sysmon started in the background (PID $$(cat $(PIDFILE)), interval $(INTERVAL))" ## stop: stop the background process (by PID file -- precise and safe) stop: diff --git a/cmd/sysmon/main.go b/cmd/sysmon/main.go new file mode 100644 index 0000000..c18042d --- /dev/null +++ b/cmd/sysmon/main.go @@ -0,0 +1,89 @@ +// Command sysmon keeps macOS "active" by posting a real mouse-move event on a +// fixed interval, which resets the system idle timer so the screen does not +// sleep. The cursor is nudged one pixel and back, so it stays effectively in +// place. +package main + +import ( + "context" + "flag" + "log/slog" + "os" + "os/signal" + "syscall" + "time" + + "sysmon/internal/activity/adapter" + "sysmon/internal/activity/domain" + "sysmon/internal/activity/service" +) + +const ( + // defaultInterval is how often the cursor is nudged by default. + defaultInterval = 10 * time.Second + // nudgeOffset is the pixel shift applied then reversed on each nudge. + nudgeOffset = 1 +) + +// Process exit codes. +const ( + exitOK = 0 + exitFailure = 1 + exitUsage = 2 +) + +func main() { + os.Exit(run()) +} + +// run wires the dependencies and drives the keeper to completion, returning +// the process exit code. It is separated from main so that deferred cleanup +// (stopping the signal-notify context) always runs before the process exits. +func run() int { + interval := flag.Duration("interval", defaultInterval, "how often to nudge the cursor (e.g. 10s, 30s, 1m)") + quiet := flag.Bool("quiet", false, "suppress routine logs; still print warnings and errors") + flag.Parse() + + level := slog.LevelInfo + if *quiet { + level = slog.LevelWarn + } + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level})) + + // Validate the settings through the domain before starting, so a bad + // -interval (e.g. zero or negative) fails fast with a clear message + // instead of panicking inside time.NewTicker. + validInterval, err := domain.NewInterval(*interval) + if err != nil { + logger.Error("invalid interval", "err", err, "value", interval.String()) + return exitUsage + } + // nudgeOffset is a positive compile-time constant, so this never errors + // today; it still goes through the domain constructor so the offset is + // validated the same way as the interval. + offset, err := domain.NewOffset(nudgeOffset) + if err != nil { + logger.Error("invalid offset", "err", err, "value", nudgeOffset) + return exitUsage + } + + // Preflight: without Accessibility permission macOS silently drops the + // synthetic mouse events, so warn once at startup rather than looking like + // a no-op. This is logged at Warn so it shows even under -quiet. + if !adapter.AccessibilityTrusted() { + logger.Warn("accessibility permission not granted; mouse events will be ignored until you enable sysmon's terminal in System Settings -> Privacy & Security -> Accessibility") + } + + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + pointer := adapter.NewCGPointer() + keeper := service.NewKeeperService(pointer, logger, validInterval, offset) + + if err := keeper.Run(ctx); err != nil { + logger.Error("sysmon exited with error", "err", err) + return exitFailure + } + + return exitOK +} diff --git a/cmd/sysmon/main_test.go b/cmd/sysmon/main_test.go new file mode 100644 index 0000000..c1a33d9 --- /dev/null +++ b/cmd/sysmon/main_test.go @@ -0,0 +1,224 @@ +//go:build darwin + +package main_test + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// syncBuffer is a mutex-guarded byte buffer used to capture a child +// process's combined output while it is still running, so a test can poll +// for a marker line (e.g. "sysmon started") instead of racing a fixed +// sleep against the process's own startup latency. +type syncBuffer struct { + mu sync.Mutex + buf strings.Builder +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// waitForOutput polls buf until substr appears or timeout elapses, failing +// the test in the latter case. It never sleeps longer than needed once the +// marker appears, so it does not slow down a fast machine or flake on a +// slow one. +func waitForOutput(t *testing.T, buf *syncBuffer, substr string, timeout time.Duration) { + t.Helper() + + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if strings.Contains(buf.String(), substr) { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("timed out waiting for %q in output; got:\n%s", substr, buf.String()) +} + +// buildOnce ensures the sysmon binary is compiled exactly once for the +// whole test binary run, however many tests in this package request it. +// Building it separately per parallel test wastes wall-clock time and, on +// a loaded machine, can push the child process's startup so late that a +// fixed-delay signal in another test fires before the binary has even +// produced its first log line. +var ( + buildOnce sync.Once + builtBin string + errBinBuild error +) + +// binPath returns the path to a compiled sysmon binary shared across all +// tests in this package. Building is skipped entirely (via t.Skip) when +// testing.Short() is set, since compiling a fresh binary is too slow for a +// quick unit-test run. +func binPath(t *testing.T) string { + t.Helper() + + if testing.Short() { + t.Skip("skipping cmd/sysmon integration test in -short mode") + } + + buildOnce.Do(func() { + _, thisFile, _, ok := runtime.Caller(0) + if !ok { + errBinBuild = os.ErrInvalid + return + } + repoRoot := filepath.Join(filepath.Dir(thisFile), "..", "..") + + dir, err := os.MkdirTemp("", "sysmon-bin-*") + if err != nil { + errBinBuild = err + return + } + bin := filepath.Join(dir, "sysmon") + + // #nosec G204 -- fixed argv (go build -o ./cmd/sysmon) built + // from this test's own repo root, not from external/user input. + cmd := exec.CommandContext(context.Background(), "go", "build", "-o", bin, "./cmd/sysmon") + cmd.Dir = repoRoot + var out []byte + out, errBinBuild = cmd.CombinedOutput() + if errBinBuild != nil { + errBinBuild = fmt.Errorf("go build failed: %s: %w", out, errBinBuild) + return + } + builtBin = bin + }) + + require.NoError(t, errBinBuild) + return builtBin +} + +// TestMain_InvalidInterval covers both flag-validation failure exit paths: +// a negative interval and a zero interval must both fail fast with exit +// code 2 (exitUsage) and a message identifying the problem. +func TestMain_InvalidInterval(t *testing.T) { + t.Parallel() + + bin := binPath(t) + + tests := []struct { + name string + arg string + wantText string + }{ + {name: "negative interval", arg: "-interval=-5s", wantText: "invalid interval"}, + {name: "zero interval", arg: "-interval=0s", wantText: "invalid interval"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // #nosec G204 -- bin is our own freshly built test binary path, + // not external/user input. + cmd := exec.CommandContext(t.Context(), bin, tt.arg) + out, err := cmd.CombinedOutput() + + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr, "expected the process to exit non-zero") + assert.Equal(t, 2, exitErr.ExitCode()) + assert.Contains(t, string(out), tt.wantText) + }) + } +} + +// TestMain_Lifecycle starts sysmon with a short interval, lets it run for a +// bit, then sends SIGTERM and waits for a clean exit. It asserts exit code 0 +// and that both the startup and shutdown log lines were emitted. No fixed +// sleep races the process: shutdown is awaited via the Wait() error channel +// with a generous timeout. +func TestMain_Lifecycle(t *testing.T) { + t.Parallel() + + bin := binPath(t) + + // #nosec G204 -- bin is our own freshly built test binary path, not + // external/user input. + cmd := exec.CommandContext(t.Context(), bin, "-interval=50ms") + out := &syncBuffer{} + cmd.Stdout = out + cmd.Stderr = out + + require.NoError(t, cmd.Start()) + + // Wait for the process to actually reach the "started" log line rather + // than sleeping a fixed duration: on a loaded machine, go build/link + // contention from sibling parallel tests can push process startup out + // far enough that a fixed sleep would race it. + waitForOutput(t, out, "sysmon started", 5*time.Second) + + require.NoError(t, cmd.Process.Signal(syscall.SIGTERM)) + + waitDone := make(chan error, 1) + go func() { waitDone <- cmd.Wait() }() + + select { + case err := <-waitDone: + require.NoError(t, err, "process should exit cleanly on SIGTERM; output:\n%s", out.String()) + case <-time.After(5 * time.Second): + _ = cmd.Process.Kill() + t.Fatalf("process did not exit within timeout after SIGTERM; output:\n%s", out.String()) + } + + output := out.String() + assert.Contains(t, output, "sysmon started", "expected startup log line, got:\n%s", output) + assert.Contains(t, output, "sysmon stopped", "expected shutdown log line, got:\n%s", output) +} + +// TestMain_ExitCodeZeroOnCleanShutdown is a narrower companion to +// TestMain_Lifecycle: it isolates the exit-code assertion using +// os.Interrupt (SIGINT), which the process also handles via +// signal.NotifyContext. It runs without -quiet (unlike a plain smoke test) +// so waitForOutput has a reliable marker to poll for before signaling. +func TestMain_ExitCodeZeroOnCleanShutdown(t *testing.T) { + t.Parallel() + + bin := binPath(t) + + // #nosec G204 -- bin is our own freshly built test binary path, not + // external/user input. + cmd := exec.CommandContext(t.Context(), bin, "-interval=50ms") + out := &syncBuffer{} + cmd.Stdout = out + cmd.Stderr = out + + require.NoError(t, cmd.Start()) + + waitForOutput(t, out, "sysmon started", 5*time.Second) + require.NoError(t, cmd.Process.Signal(os.Interrupt)) + + waitDone := make(chan error, 1) + go func() { waitDone <- cmd.Wait() }() + + select { + case err := <-waitDone: + require.NoError(t, err, "process should exit 0 on SIGINT; output:\n%s", out.String()) + case <-time.After(5 * time.Second): + _ = cmd.Process.Kill() + t.Fatalf("process did not exit within timeout after SIGINT; output:\n%s", out.String()) + } +} diff --git a/go.mod b/go.mod index 2e528ed..c24a302 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,14 @@ module sysmon go 1.26.5 + +require ( + github.com/stretchr/testify v1.11.1 + go.uber.org/mock v0.6.0 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..fd98840 --- /dev/null +++ b/go.sum @@ -0,0 +1,12 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= +go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/activity/adapter/cgpointer.go b/internal/activity/adapter/cgpointer.go new file mode 100644 index 0000000..daea01f --- /dev/null +++ b/internal/activity/adapter/cgpointer.go @@ -0,0 +1,73 @@ +//go:build darwin + +// Package adapter contains the concrete, platform-specific implementations +// of the activity hexagon's ports. This is the only package permitted to +// import "C" / cgo. +package adapter + +/* +#cgo LDFLAGS: -framework ApplicationServices -framework CoreGraphics +#include + +// nudge moves the cursor by (dx, dy) from its current position and posts a real +// MouseMoved event -- that event is what resets the system idle timer, unlike a +// plain cursor reposition. It returns 0 on success, or -1 if CoreGraphics could +// not create an event (leaving the cursor untouched), so it never releases a +// NULL reference. +static int nudge(int dx, int dy) { + CGEventRef cur = CGEventCreate(NULL); + if (cur == NULL) { + return -1; + } + CGPoint p = CGEventGetLocation(cur); + CFRelease(cur); + + CGPoint np = CGPointMake(p.x + dx, p.y + dy); + CGEventRef move = CGEventCreateMouseEvent(NULL, kCGEventMouseMoved, np, kCGMouseButtonLeft); + if (move == NULL) { + return -1; + } + CGEventPost(kCGHIDEventTap, move); + CFRelease(move); + return 0; +} + +// accessibilityTrusted reports whether this process is trusted for the macOS +// Accessibility API. Without trust, posted mouse events are silently dropped. +static int accessibilityTrusted(void) { + return AXIsProcessTrusted() ? 1 : 0; +} +*/ +import "C" + +import "errors" + +// ErrNudgeFailed is returned when CoreGraphics could not create the event +// needed to move the pointer. The cursor is left untouched in that case. +var ErrNudgeFailed = errors.New("coregraphics failed to create mouse event") + +// CGPointer implements port.Pointer using CoreGraphics on macOS. +type CGPointer struct{} + +// NewCGPointer constructs a CGPointer. +func NewCGPointer() *CGPointer { + return &CGPointer{} +} + +// Nudge moves the system pointer by (dx, dy) and posts a real MouseMoved event +// via CoreGraphics. It returns ErrNudgeFailed if the event could not be created +// (the cursor is left untouched in that case). +func (c *CGPointer) Nudge(dx, dy int) error { + if C.nudge(C.int(dx), C.int(dy)) != 0 { + return ErrNudgeFailed + } + return nil +} + +// AccessibilityTrusted reports whether this process has macOS Accessibility +// permission. When false, synthetic mouse events are silently dropped and the +// idle timer is never reset, so callers should warn the user to grant access in +// System Settings -> Privacy & Security -> Accessibility. +func AccessibilityTrusted() bool { + return C.accessibilityTrusted() != 0 +} diff --git a/internal/activity/adapter/cgpointer_test.go b/internal/activity/adapter/cgpointer_test.go new file mode 100644 index 0000000..0da2717 --- /dev/null +++ b/internal/activity/adapter/cgpointer_test.go @@ -0,0 +1,50 @@ +//go:build darwin + +package adapter_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sysmon/internal/activity/adapter" +) + +// TestNewCGPointer asserts construction never returns nil. +func TestNewCGPointer(t *testing.T) { + t.Parallel() + + p := adapter.NewCGPointer() + + require.NotNil(t, p) +} + +// TestCGPointer_Nudge_ZeroOffset is a non-disruptive smoke test: a (0, 0) +// nudge posts a real MouseMoved event at the cursor's current location, so +// the cursor does not visibly move regardless of whether the test process +// has macOS Accessibility permission. CoreGraphics can still create and post +// the event without that permission (the event is only silently dropped by +// the system on delivery), so this call is expected to succeed in CI. +func TestCGPointer_Nudge_ZeroOffset(t *testing.T) { + t.Parallel() + + p := adapter.NewCGPointer() + + err := p.Nudge(0, 0) + + require.NoError(t, err) +} + +// TestAccessibilityTrusted asserts the accessor returns a bool without +// panicking. The actual value depends on whether the test runner's terminal +// has been granted Accessibility permission, which CI cannot guarantee, so +// this test intentionally does not assert true or false. +func TestAccessibilityTrusted(t *testing.T) { + t.Parallel() + + require.NotPanics(t, func() { + got := adapter.AccessibilityTrusted() + assert.IsType(t, false, got) + }) +} diff --git a/internal/activity/domain/activity.go b/internal/activity/domain/activity.go new file mode 100644 index 0000000..1ee162a --- /dev/null +++ b/internal/activity/domain/activity.go @@ -0,0 +1,56 @@ +// Package domain contains the pure value objects and validation rules for the +// activity-keeping use case. It imports nothing from port, service, or +// adapter, and has no cgo or os dependency, so it builds and tests on any +// platform. +package domain + +import ( + "errors" + "time" +) + +// ErrNonPositiveInterval is returned when a non-positive duration is supplied +// to NewInterval. +var ErrNonPositiveInterval = errors.New("interval must be greater than zero") + +// ErrNonPositiveOffset is returned when a non-positive pixel value is +// supplied to NewOffset. +var ErrNonPositiveOffset = errors.New("offset must be greater than zero") + +// Interval is a validated positive duration between nudges. +type Interval struct { + d time.Duration +} + +// NewInterval validates d and returns an Interval wrapping it. It rejects any +// duration that is zero or negative, preventing a busy-loop or a panic from a +// non-positive time.Ticker. +func NewInterval(d time.Duration) (Interval, error) { + if d <= 0 { + return Interval{}, ErrNonPositiveInterval + } + return Interval{d: d}, nil +} + +// Duration returns the wrapped time.Duration. +func (i Interval) Duration() time.Duration { + return i.d +} + +// Offset is the validated pixel shift magnitude applied then reversed on +// each nudge. +type Offset int + +// NewOffset validates px and returns an Offset wrapping it. It rejects any +// value that is zero or negative. +func NewOffset(px int) (Offset, error) { + if px <= 0 { + return 0, ErrNonPositiveOffset + } + return Offset(px), nil +} + +// Int returns the wrapped pixel value. +func (o Offset) Int() int { + return int(o) +} diff --git a/internal/activity/domain/activity_test.go b/internal/activity/domain/activity_test.go new file mode 100644 index 0000000..d66d447 --- /dev/null +++ b/internal/activity/domain/activity_test.go @@ -0,0 +1,125 @@ +package domain_test + +import ( + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sysmon/internal/activity/domain" +) + +func TestNewInterval(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + d time.Duration + wantErr bool + }{ + {name: "positive duration is accepted", d: 10 * time.Second, wantErr: false}, + {name: "zero duration is rejected", d: 0, wantErr: true}, + {name: "negative duration is rejected", d: -time.Second, wantErr: true}, + {name: "one nanosecond is accepted (boundary)", d: 1 * time.Nanosecond, wantErr: false}, + {name: "very large duration is accepted (boundary)", d: math.MaxInt64, wantErr: false}, + {name: "math.MinInt64 duration is rejected (boundary)", d: math.MinInt64, wantErr: true}, + {name: "large negative duration is rejected", d: -24 * time.Hour, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := domain.NewInterval(tt.d) + + if tt.wantErr { + require.ErrorIs(t, err, domain.ErrNonPositiveInterval) + assert.Equal(t, domain.Interval{}, got) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.d, got.Duration()) + }) + } +} + +func TestNewOffset(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + px int + wantErr bool + }{ + {name: "positive offset is accepted", px: 1, wantErr: false}, + {name: "zero offset is rejected", px: 0, wantErr: true}, + {name: "negative offset is rejected", px: -1, wantErr: true}, + {name: "very large offset is accepted (boundary)", px: math.MaxInt, wantErr: false}, + {name: "math.MinInt offset is rejected (boundary)", px: math.MinInt, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := domain.NewOffset(tt.px) + + if tt.wantErr { + require.ErrorIs(t, err, domain.ErrNonPositiveOffset) + assert.Equal(t, domain.Offset(0), got) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.px, got.Int()) + }) + } +} + +// TestInterval_Duration_RoundTrip asserts that Duration() returns exactly the +// value passed to NewInterval, with no rounding or truncation. +func TestInterval_Duration_RoundTrip(t *testing.T) { + t.Parallel() + + values := []time.Duration{ + 1 * time.Nanosecond, + 1, + 40 * time.Millisecond, + 10 * time.Second, + 365 * 24 * time.Hour, + math.MaxInt64, + } + + for _, d := range values { + iv, err := domain.NewInterval(d) + require.NoError(t, err) + assert.Equal(t, d, iv.Duration()) + } +} + +// TestOffset_Int_RoundTrip asserts that Int() returns exactly the pixel value +// passed to NewOffset. +func TestOffset_Int_RoundTrip(t *testing.T) { + t.Parallel() + + values := []int{1, 2, 40, 1000, math.MaxInt} + + for _, px := range values { + off, err := domain.NewOffset(px) + require.NoError(t, err) + assert.Equal(t, px, off.Int()) + } +} + +// TestErrorsAreDistinct guards against a future refactor accidentally +// aliasing the two sentinel errors, which would make errors.Is checks +// ambiguous between Interval and Offset validation failures. +func TestErrorsAreDistinct(t *testing.T) { + t.Parallel() + + require.NotErrorIs(t, domain.ErrNonPositiveInterval, domain.ErrNonPositiveOffset) + require.NotErrorIs(t, domain.ErrNonPositiveOffset, domain.ErrNonPositiveInterval) +} diff --git a/internal/activity/port/keeper.go b/internal/activity/port/keeper.go new file mode 100644 index 0000000..a589674 --- /dev/null +++ b/internal/activity/port/keeper.go @@ -0,0 +1,8 @@ +package port + +import "context" + +// Keeper runs the activity-keeping loop until ctx is done. +type Keeper interface { + Run(ctx context.Context) error +} diff --git a/internal/activity/port/mock/mock_pointer.go b/internal/activity/port/mock/mock_pointer.go new file mode 100644 index 0000000..afbd82a --- /dev/null +++ b/internal/activity/port/mock/mock_pointer.go @@ -0,0 +1,54 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: sysmon/internal/activity/port (interfaces: Pointer) +// +// Generated by this command: +// +// mockgen -destination=mock/mock_pointer.go -package=mock sysmon/internal/activity/port Pointer +// + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockPointer is a mock of Pointer interface. +type MockPointer struct { + ctrl *gomock.Controller + recorder *MockPointerMockRecorder + isgomock struct{} +} + +// MockPointerMockRecorder is the mock recorder for MockPointer. +type MockPointerMockRecorder struct { + mock *MockPointer +} + +// NewMockPointer creates a new mock instance. +func NewMockPointer(ctrl *gomock.Controller) *MockPointer { + mock := &MockPointer{ctrl: ctrl} + mock.recorder = &MockPointerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPointer) EXPECT() *MockPointerMockRecorder { + return m.recorder +} + +// Nudge mocks base method. +func (m *MockPointer) Nudge(dx, dy int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Nudge", dx, dy) + ret0, _ := ret[0].(error) + return ret0 +} + +// Nudge indicates an expected call of Nudge. +func (mr *MockPointerMockRecorder) Nudge(dx, dy any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Nudge", reflect.TypeOf((*MockPointer)(nil).Nudge), dx, dy) +} diff --git a/internal/activity/port/pointer.go b/internal/activity/port/pointer.go new file mode 100644 index 0000000..29f3293 --- /dev/null +++ b/internal/activity/port/pointer.go @@ -0,0 +1,12 @@ +// Package port contains the driven and driving interfaces of the activity +// hexagon. Ports know nothing about concrete implementations: no cgo, no +// adapter import. +package port + +//go:generate mockgen -destination=mock/mock_pointer.go -package=mock sysmon/internal/activity/port Pointer + +// Pointer abstracts moving the system pointer by a relative offset and +// posting a synthetic mouse-moved event, which resets the OS idle timer. +type Pointer interface { + Nudge(dx, dy int) error +} diff --git a/internal/activity/service/keeper_service.go b/internal/activity/service/keeper_service.go new file mode 100644 index 0000000..530a4fb --- /dev/null +++ b/internal/activity/service/keeper_service.go @@ -0,0 +1,95 @@ +// Package service contains the KeeperService use case: the ticker loop that +// drives Pointer.Nudge on a fixed interval. It depends only on domain and +// port, never on adapter. +package service + +import ( + "context" + "log/slog" + "time" + + "sysmon/internal/activity/domain" + "sysmon/internal/activity/port" +) + +// nudgePause is the delay between the forward and backward nudge of a single +// cycle. It is an implementation detail of the there-and-back move, not a +// user-facing setting, so it is a package constant rather than a parameter. +const nudgePause = 40 * time.Millisecond + +// KeeperService implements port.Keeper: it drives a ticker loop that nudges +// the pointer there and back on each tick, and stops cleanly when ctx is +// cancelled. +type KeeperService struct { + pointer port.Pointer + logger *slog.Logger + interval domain.Interval + offset domain.Offset +} + +// NewKeeperService constructs a KeeperService. pointer and logger are injected +// dependencies (never package globals), which is what makes this service +// unit-testable without a real mouse or macOS Accessibility. interval and +// offset are validated domain value objects, so the ticker can never receive a +// non-positive duration and the two settings cannot be swapped by accident. +func NewKeeperService(pointer port.Pointer, logger *slog.Logger, interval domain.Interval, offset domain.Offset) *KeeperService { + return &KeeperService{ + pointer: pointer, + logger: logger, + interval: interval, + offset: offset, + } +} + +// Run starts the nudge ticker loop and blocks until ctx is done. On each tick +// it nudges the pointer forward by offset, pauses, then nudges it back by the +// same offset (there-and-back), so the cursor stays effectively in place while +// a real move event is posted. Run returns nil on clean cancellation. +func (k *KeeperService) Run(ctx context.Context) error { + ticker := time.NewTicker(k.interval.Duration()) + defer ticker.Stop() + + offset := k.offset.Int() + k.logger.Info("sysmon started", "interval", k.interval.Duration()) + + for { + select { + case <-ticker.C: + k.nudgeCycle(ctx, offset) + // nudgeCycle has already run its compensating back-nudge (even if + // ctx was cancelled during the pause). If ctx is now done, stop + // here instead of waiting for the next tick. + if ctx.Err() != nil { + k.logger.Info("sysmon stopped") + return nil + } + case <-ctx.Done(): + k.logger.Info("sysmon stopped") + return nil + } + } +} + +// nudgeCycle performs one there-and-back nudge. The pause between the two +// nudges is interruptible: if ctx is cancelled during it, the pause ends early +// but the compensating back-nudge STILL runs, so the cursor returns to its +// original position before shutdown. The only path that skips the back-nudge +// is a failed forward nudge (the cursor never moved). +func (k *KeeperService) nudgeCycle(ctx context.Context, offset int) { + if err := k.pointer.Nudge(offset, 0); err != nil { + // The forward nudge failed, so the cursor never moved -- do NOT send + // the compensating back-nudge, which would leave it offset the other + // way. Skipping it keeps the cursor where it was. + k.logger.Error("nudge failed", "err", err) + return + } + + select { + case <-time.After(nudgePause): + case <-ctx.Done(): + } + + if err := k.pointer.Nudge(-offset, 0); err != nil { + k.logger.Error("nudge back failed", "err", err) + } +} diff --git a/internal/activity/service/keeper_service_test.go b/internal/activity/service/keeper_service_test.go new file mode 100644 index 0000000..9817a0a --- /dev/null +++ b/internal/activity/service/keeper_service_test.go @@ -0,0 +1,332 @@ +package service_test + +import ( + "context" + "errors" + "log/slog" + "sync" + "testing" + "testing/synctest" + "time" + + "sysmon/internal/activity/domain" + "sysmon/internal/activity/port/mock" + "sysmon/internal/activity/service" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +// errNudge is a local sentinel used to simulate a Pointer failure in these +// tests. It must never be adapter.ErrNudgeFailed: service tests stay +// cgo-free and must not import internal/activity/adapter. +var errNudge = errors.New("nudge sentinel failure") + +// mustInterval builds a validated domain.Interval, failing the test on error. +func mustInterval(t *testing.T, d time.Duration) domain.Interval { + t.Helper() + iv, err := domain.NewInterval(d) + require.NoError(t, err) + return iv +} + +// mustOffset builds a validated domain.Offset, failing the test on error. +func mustOffset(t *testing.T, px int) domain.Offset { + t.Helper() + off, err := domain.NewOffset(px) + require.NoError(t, err) + return off +} + +func TestKeeperService_NudgesPerTick(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + const ( + interval = 10 * time.Second + ticks = 3 + ) + + gomock.InOrder( + pointer.EXPECT().Nudge(1, 0).Return(nil), + pointer.EXPECT().Nudge(-1, 0).Return(nil), + pointer.EXPECT().Nudge(1, 0).Return(nil), + pointer.EXPECT().Nudge(-1, 0).Return(nil), + pointer.EXPECT().Nudge(1, 0).Return(nil), + pointer.EXPECT().Nudge(-1, 0).Return(nil), + ) + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, interval), mustOffset(t, 1)) + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(ticks * interval) + synctest.Wait() + + cancel() + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +// TestKeeperService_CompensatesOnCancelDuringPause exercises the exact path +// added by review #1: cancellation landing strictly inside the inter-nudge +// pause. The forward nudge triggers cancel(), so the pause select takes the +// ctx.Done arm, and the compensating back-nudge must still fire before Run +// returns. +func TestKeeperService_CompensatesOnCancelDuringPause(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + ctx, cancel := context.WithCancel(t.Context()) + + gomock.InOrder( + pointer.EXPECT().Nudge(1, 0).DoAndReturn(func(_, _ int) error { + cancel() // cancel while we are about to enter the pause + return nil + }), + pointer.EXPECT().Nudge(-1, 0).Return(nil), // must still compensate + ) + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, time.Second), mustOffset(t, 1)) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(time.Second) // let exactly one tick fire + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +func TestKeeperService_StopsOnContextCancel(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, time.Second), mustOffset(t, 1)) + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + synctest.Wait() + cancel() + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +// TestKeeperService_ForwardNudgeFailureSkipsBackNudge exercises nudgeCycle's +// early-return path: when the forward nudge fails, the cursor never moved, +// so the compensating back-nudge (-1, 0) must NOT be sent. The failing call +// also cancels ctx (via DoAndReturn) so Run observes ctx.Err() != nil right +// after nudgeCycle returns and stops without waiting for another tick. +func TestKeeperService_ForwardNudgeFailureSkipsBackNudge(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + ctx, cancel := context.WithCancel(t.Context()) + + pointer.EXPECT().Nudge(1, 0).DoAndReturn(func(_, _ int) error { + cancel() + return errNudge + }) + // No expectation for Nudge(-1, 0): gomock's controller will fail the + // test if it is called, since only the forward nudge was recorded. + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, time.Second), mustOffset(t, 1)) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(time.Second) // let exactly one tick fire + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +// TestKeeperService_BackNudgeFailureIsTolerated asserts that a failed back +// nudge is logged but does not stop the loop: the second tick still runs +// both nudges normally. +func TestKeeperService_BackNudgeFailureIsTolerated(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + ctx, cancel := context.WithCancel(t.Context()) + + gomock.InOrder( + pointer.EXPECT().Nudge(1, 0).Return(nil), + pointer.EXPECT().Nudge(-1, 0).Return(errNudge), + pointer.EXPECT().Nudge(1, 0).Return(nil), + pointer.EXPECT().Nudge(-1, 0).DoAndReturn(func(_, _ int) error { + cancel() + return nil + }), + ) + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, time.Second), mustOffset(t, 1)) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(2 * time.Second) // let two ticks fire + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +// TestKeeperService_CustomOffset asserts that a non-default Offset value is +// forwarded verbatim to Pointer.Nudge in both directions. +func TestKeeperService_CustomOffset(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + ctx, cancel := context.WithCancel(t.Context()) + + gomock.InOrder( + pointer.EXPECT().Nudge(3, 0).Return(nil), + pointer.EXPECT().Nudge(-3, 0).DoAndReturn(func(_, _ int) error { + cancel() + return nil + }), + ) + + logger := slog.New(slog.DiscardHandler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, time.Second), mustOffset(t, 3)) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(time.Second) + synctest.Wait() + + require.NoError(t, <-done) + }) +} + +// capturingHandler is a minimal, race-safe slog.Handler that records every +// Handle call so tests can assert on emitted log records. It is not a +// hand-written fake of a domain dependency (which the test stack forbids) -- +// it is a real slog.Handler implementation, the mechanism slog itself +// documents for testing loggers. +type capturingHandler struct { + mu *sync.Mutex + records *[]slog.Record +} + +func newCapturingHandler() *capturingHandler { + return &capturingHandler{mu: &sync.Mutex{}, records: &[]slog.Record{}} +} + +func (h *capturingHandler) Enabled(context.Context, slog.Level) bool { return true } + +func (h *capturingHandler) Handle(_ context.Context, r slog.Record) error { + h.mu.Lock() + defer h.mu.Unlock() + *h.records = append(*h.records, r) + return nil +} + +func (h *capturingHandler) WithAttrs([]slog.Attr) slog.Handler { return h } + +func (h *capturingHandler) WithGroup(string) slog.Handler { return h } + +func (h *capturingHandler) snapshot() []slog.Record { + h.mu.Lock() + defer h.mu.Unlock() + out := make([]slog.Record, len(*h.records)) + copy(out, *h.records) + return out +} + +// TestKeeperService_LogsStartAndStop covers LOG-01: Run must log an Info +// "sysmon started" record carrying the configured interval as an attribute, +// then an Info "sysmon stopped" record once ctx is cancelled. +func TestKeeperService_LogsStartAndStop(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + ctrl := gomock.NewController(t) + pointer := mock.NewMockPointer(ctrl) + + const interval = time.Second + + ctx, cancel := context.WithCancel(t.Context()) + + pointer.EXPECT().Nudge(1, 0).Return(nil) + pointer.EXPECT().Nudge(-1, 0).DoAndReturn(func(_, _ int) error { + cancel() + return nil + }) + + handler := newCapturingHandler() + logger := slog.New(handler) + svc := service.NewKeeperService(pointer, logger, mustInterval(t, interval), mustOffset(t, 1)) + + done := make(chan error, 1) + go func() { done <- svc.Run(ctx) }() + + time.Sleep(interval) + synctest.Wait() + + require.NoError(t, <-done) + + records := handler.snapshot() + require.NotEmpty(t, records) + + var sawStarted, sawStopped bool + for _, r := range records { + switch r.Message { + case "sysmon started": + sawStarted = true + assert.Equal(t, slog.LevelInfo, r.Level) + + var sawIntervalAttr bool + r.Attrs(func(a slog.Attr) bool { + if a.Key == "interval" { + sawIntervalAttr = true + } + return true + }) + assert.True(t, sawIntervalAttr, "expected an interval attribute on the started record") + case "sysmon stopped": + sawStopped = true + assert.Equal(t, slog.LevelInfo, r.Level) + } + } + + assert.True(t, sawStarted, "expected a sysmon started record") + assert.True(t, sawStopped, "expected a sysmon stopped record") + }) +} diff --git a/main.go b/main.go deleted file mode 100644 index e87a820..0000000 --- a/main.go +++ /dev/null @@ -1,77 +0,0 @@ -// Command sysmon keeps macOS "active" by posting a real mouse-move event on a -// fixed interval, which resets the system idle timer so the screen does not -// sleep. The cursor is nudged one pixel and back, so it stays effectively in -// place. -package main - -/* -#cgo LDFLAGS: -framework ApplicationServices -framework CoreGraphics -#include - -// nudge moves the cursor by (dx, dy) from its current position and posts a real -// MouseMoved event -- that event is what resets the system idle timer, unlike a -// plain cursor reposition. -static void nudge(int dx, int dy) { - CGEventRef cur = CGEventCreate(NULL); - CGPoint p = CGEventGetLocation(cur); - CFRelease(cur); - - CGPoint np = CGPointMake(p.x + dx, p.y + dy); - CGEventRef move = CGEventCreateMouseEvent(NULL, kCGEventMouseMoved, np, kCGMouseButtonLeft); - CGEventPost(kCGHIDEventTap, move); - CFRelease(move); -} -*/ -import "C" - -import ( - "flag" - "log" - "os" - "os/signal" - "syscall" - "time" -) - -const ( - // defaultInterval is how often the cursor is nudged by default. - defaultInterval = 10 * time.Second - // nudgePause is the delay between the forward and backward nudge. - nudgePause = 40 * time.Millisecond -) - -func main() { - interval := flag.Duration("interval", defaultInterval, "how often to nudge the cursor (e.g. 10s, 30s, 1m)") - quiet := flag.Bool("quiet", false, "do not write logs to stdout") - flag.Parse() - - if *quiet { - log.SetOutput(os.Stderr) - } - - sig := make(chan os.Signal, 1) - signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) - - ticker := time.NewTicker(*interval) - defer ticker.Stop() - - if !*quiet { - log.Printf("sysmon: active every %s (Ctrl+C to quit)", *interval) - } - - for { - select { - case <-ticker.C: - // Small offset there and back: the cursor stays effectively in - // place, but the system sees movement. - C.nudge(1, 0) - time.Sleep(nudgePause) - C.nudge(-1, 0) - case <-sig: - if !*quiet { - log.Println("sysmon: stopped") - } - return - } - } -}