Repository navigation
Phase 1: hexagonal refactor (ports & adapters) + tests - #2
Merged
Merged
Conversation
Add internal/activity/domain with validated Interval (wraps time.Duration, rejects non-positive) and Offset (wraps int, rejects non-positive) value types, per ARCH-04. Package has zero cgo/os dependency and builds under GOOS=linux. Covered by table-driven tests using testify assertions. Pulled the testify go.mod dependency forward from Task 3 since the domain test needs it to compile; go.mod/go.sum updated via go mod tidy.
Add internal/activity/port with the driven port Pointer (Nudge(dx, dy int) error) and the driving port Keeper (Run(ctx context.Context) error), per ARCH-02/ARCH-03. Ports import nothing from adapter. Add a committed //go:generate mockgen directive in pointer.go targeting the Pointer interface, generating into port/mock (package mock) as the locked test stack requires a generated mock, not a hand-written fake.
Add go.uber.org/mock as a test-only dependency and generate internal/activity/port/mock/mock_pointer.go via mockgen, driven by the //go:generate directive in pointer.go. The generated file carries the standard DO NOT EDIT header and is exempt from lint under exclusions.generated: strict. Switched the go:generate directive to invoke the installed mockgen binary directly (rather than `go run go.uber.org/mock/mockgen`), since running mockgen as a tool dependency via `go run` requires its full transitive dependency graph in go.sum for no benefit here. go.mod/go.sum updated via go mod tidy: testify and go.uber.org/mock are test-only requires, never linked into the production binary.
Add internal/activity/service.KeeperService implementing port.Keeper: a ticker loop that nudges the pointer forward then back on each tick with a configurable pause, and stops cleanly on context cancellation. Logger and Pointer are constructor-injected, never package globals, per LOG-01 and CONTEXT.md. Depends only on domain and port, never adapter. Add keeper_service_test.go covering nudges-per-tick and stop-on-cancel using testing/synctest for a deterministic fake clock, the mockgen Pointer mock with gomock call ordering, and testify assertions. Tests run silent (slog.DiscardHandler) and never touch the darwin adapter.
Add internal/activity/adapter.CGPointer implementing port.Pointer via cgo/CoreGraphics, per ARCH-02. The file leads with //go:build darwin followed by a blank line, then the package clause. The C nudge() function body -- including both CFRelease(cur) and CFRelease(move) calls -- is copied verbatim from the original main.go (diff-verified byte-for-byte identical) to avoid reintroducing a fixed CoreFoundation object leak in this long-running process. The #cgo LDFLAGS and #include lines are unchanged. This is the only package permitted to import "C"/cgo.
Add cmd/sysmon/main.go as the new wiring-only entry point (ARCH-01, ARCH-05): parses -interval/-quiet with the same names/usage/defaults as before (BUILD-02), builds a slog.TextHandler logger whose level is raised to LevelWarn under -quiet (not a stream redirect -- LOG-01), replaces the manual signal.Notify/select plumbing with signal.NotifyContext(SIGINT, SIGTERM), constructs adapter.NewCGPointer() and service.NewKeeperService via manual DI, and calls keeper.Run(ctx). Removes the flat root main.go. Fix .gitignore: the bare `sysmon` pattern (meant to ignore the built root binary) also matched the new cmd/sysmon/ directory anywhere in the tree, silently blocking it from being tracked. Scoped the ignore to /sysmon (repo-root binary only) so cmd/sysmon/ is tracked normally. Manually verified: builds, runs, logs "started"/"stopped", -quiet produces no stdout output, and SIGTERM yields a clean exit code 0.
Change the Makefile build target to `go build -o $(BINARY) ./cmd/sysmon` now that main.go lives under cmd/sysmon; all other targets are unchanged (BUILD-01). Fix real issues surfaced by the full quality gate (go build ./..., golangci-lint run ./..., go test -race ./...): - gocritic exitAfterDefer in cmd/sysmon/main.go: os.Exit(1) was called directly inside main(), which skips the deferred stop() for the signal-notify context. Split main() into a thin os.Exit(run()) wrapper plus a run() that returns an exit code, so deferred cleanup always runs before the process exits. - golangci-lint's gofumpt formatter misformatted files that import our own sysmon/... packages, because formatters.settings.gofumpt lacked module-path -- without it, gofumpt cannot distinguish local-module imports from third-party ones during import grouping. Added `module-path: sysmon` to .golangci.yml (this only started mattering once cross-package internal imports existed, i.e. with this refactor). Also fixed keeper_service_test.go's import order (local packages before third-party) to match goimports local-prefixes. `make build` now produces ./sysmon from ./cmd/sysmon; go build ./..., golangci-lint run ./... (0 issues), and go test -race ./... all pass.
Document Tasks 1-7 of the hexagonal refactor plan: files created/ modified, task commits, decisions, and four auto-fixed deviations (testify pulled forward, .gitignore blocking cmd/sysmon, gocritic exitAfterDefer, gofumpt module-path). Task 8 (human-verify checkpoint) is explicitly called out as pending.
…nudge pause Addresses code review #1 (CR-01, WR-01, WR-02, IN-01, IN-02): - wire domain.Interval/Offset into cmd + KeeperService so a non-positive -interval exits with code 2 instead of panicking in time.NewTicker - typed constructor params remove the two-duration swap hazard; nudgePause is now a service-internal constant - pause between nudges is interruptible via ctx so shutdown is prompt - document that a failed forward nudge intentionally skips the back-nudge
Addresses code review #2 (WR-01, IN-01): - add TestKeeperService_CompensatesOnCancelDuringPause: forward nudge triggers cancel() so the pause select takes ctx.Done, asserting the compensating back-nudge still fires and Run returns - clarify nudgeCycle/Run comments: back-nudge still runs after an interrupted pause; only a failed forward nudge skips it
Addresses custom-prompt review #1 (P2 findings): - cgo nudge null-checks CGEventCreate/CGEventCreateMouseEvent and returns a status; CGPointer.Nudge now returns ErrNudgeFailed instead of always nil - add AccessibilityTrusted() (AXIsProcessTrusted) and warn once at startup when permission is missing, so a silent no-op is explained - clarify NewOffset validation goes through the domain by design
Addresses custom-prompt review #2: -quiet raises the log level to Warn (it does not silence stdout), so warnings/errors incl. the accessibility preflight still print. Reword the flag help accordingly.
- domain: errors.Is sentinel checks, boundary values (1ns, 1px, MaxInt, MinInt), Duration/Int round-trips - service: forward-nudge failure skips back-nudge, back-nudge failure tolerated, custom offset, and LOG-01 start/stop logging via a capturing slog.Handler - adapter (darwin): CGPointer smoke test with zero-offset nudge and AccessibilityTrusted probe - cmd (darwin): black-box integration -- invalid-interval exit code 2, and SIGTERM/SIGINT lifecycle asserting clean startup/shutdown domain and service coverage 100%; service tests stay cgo-free (GOOS=linux).
Lets the nudge interval be overridden without editing the file, e.g. make start INTERVAL=3s. Defaults to 10s.
14 tasks done
dezween
added a commit
that referenced
this pull request
Jul 12, 2026
* docs: add task spec for realistic random cursor roaming * docs(quick): plan realistic random cursor roaming * feat(quick-260712-roaming-01): add pure domain movement planner with deterministic tests - Planner.Plan interpolates from current position to a fresh uniform random target, clamped to screen bounds - randomness injected via a minimal RandomSource seam so tests are deterministic - path is continuous, ends exactly at target, covers corner/1x1/target-equals-current edge cases - cgo/os-free, builds under GOOS=linux * feat(quick-260712-roaming-02): reshape Pointer port for absolute roaming and regenerate mock - replace Nudge(dx, dy int) error with MoveTo(x, y int) error, Position(), and Bounds() - regenerate mockgen mock from the new interface (MoveTo/Position/Bounds) Note: this leaves the tree non-building until the service/adapter/main cutover in the following tasks, as planned. * feat(quick-260712-roaming-03): rewrite KeeperService to roam once per tick - roam(ctx): read Position/Bounds, ask domain.Planner for a glide path, then drive() calls MoveTo per path point with a short inter-step pause - ctx checked between every step so cancellation stops promptly mid-glide - Position/Bounds/MoveTo errors are logged and abandon the current roam without crashing the loop - tests rewritten with synctest + regenerated mockgen mock: one roam per tick, ordered MoveTo, mid-glide cancel, per-call error handling, start/stop logging Note: this leaves the tree non-building until the adapter/main cutover in the following tasks, as planned. * feat(quick-260712-roaming-04): add CoreGraphics Position/Bounds/MoveTo to CGPointer - getPosition/getBounds/moveTo C helpers replace the relative nudge helper: current cursor location via CGEventGetLocation, main-display bounds via CGDisplayBounds(CGMainDisplayID()), absolute move via a MouseMoved event - every CoreFoundation object created is released; NULL-create failures release nothing and signal failure - ErrNudgeFailed split into ErrPositionFailed and ErrMoveFailed - compile-time assertion that *CGPointer satisfies port.Pointer - tests updated to the new API (position/bounds/move-to-current-position smoke tests, no CI mouse movement) * feat(quick-260712-roaming-05): wire main.go for random roaming and remove dead Offset domain code - construct a runtime-seeded *math/rand/v2.Rand (rand.NewPCG from the auto-seeded package generator) and a domain.Planner, wired into KeeperService alongside the pointer/logger/interval - remove nudgeOffset constant and the domain.NewOffset validation - update flag help text and package doc comment to describe random roaming instead of the 1px nudge - keep the AccessibilityTrusted preflight warning and SIGINT/SIGTERM clean-exit-0 signal handling unchanged - delete domain.Offset, NewOffset, and ErrNonPositiveOffset (and their tests): roaming uses absolute clamped targets, so a relative offset value object no longer has meaning - fix a stale ErrNudgeFailed reference in a service test comment * docs(quick): add roaming task summary * fix(roaming): clamp out-of-bounds start, reuse glide timer, clarify docs Addresses roaming code review #1: - CR-01: clamp the starting cursor position into the display bounds before interpolating, so a cursor on a secondary monitor (global coords) cannot send early glide steps off-screen; covered by a new planner test - WR-01: reuse a single time.Timer across the glide instead of time.After per step - WR-02/IN-01/IN-02: clarify rounding, bounds-per-roam, and adapter Bounds contract * test(roaming): fix orphaned comment, cover exact clamp boundary Addresses roaming code review #2 (IN-01, IN-02): restore the OnePixelScreen doc comment to its function and make the clamp test table-driven, adding the exact boundary case (curX == width). * refine(roaming): start glide timer stopped; assert in-bounds moves at service seam Addresses custom-prompt review #1 (P2): - start the reused glide timer stopped so the first inter-step gap is exactly stepPause - add a service synctest asserting every MoveTo stays within [0,w)x[0,h) even when Position() reports an out-of-bounds start * test(roaming): cover roam error paths and add planner edge cases - service: invalid-bounds (Plan error) abandon path and move-error-on-first-step abandon -- service coverage now 100% - planner: very large screen, bounded step delta corner-to-corner, and different-seed-different-path cases --------- Co-authored-by: dezween <dezween@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restructure the flat root
main.gointo a hexagonal (ports & adapters) layout, add structured logging vialog/slog, and cover the new code with tests -- with no change to observable runtime behavior.Type of change
What changed
cmd/sysmon/main.go-- entry point: flags,signal.NotifyContext, manual DI,-quietraises slog level, accessibility preflight warning. Rootmain.goremoved.internal/activity/domain-- pureInterval/Offsetvalue objects with validation (no cgo/os; builds under GOOS=linux).internal/activity/port--PointerandKeeperinterfaces +//go:generate mockgendirective.internal/activity/service--KeeperService: ticker loop, injected*slog.Logger, ctx cancellation, interruptible nudge pause. Depends only on domain + port.internal/activity/adapter--CGPointercgo/CoreGraphics behind//go:build darwin; null-checked, returnsErrNudgeFailed; bothCFReleasecalls preserved.testing/synctest.Makefilebuild target ->./cmd/sysmon.Why / context
How it was tested
go build ./...go test -race ./...(domain + service 100%; adapter/cmd covered)golangci-lint run ./...-- 0 issuesGOOS=linux go build ./internal/activity/{domain,port,service}/...-- cgo-freesysmonduntouched)Checklist
Architecture & code
macOS / cgo
CFReleasecalls preserved; null-checked event creation//go:build darwinAccessibilityTrusted)Concurrency & resources
time.Tickersleep, no busy-waitSafety & hygiene
pkill; pidfile-based Makefile control-intervalfails fast (exit 2), no panicRisks & rollback
Low -- pure refactor with behavior preserved and 100% domain/service test coverage. Revert the branch to roll back.
Out of scope / follow-ups
make start INTERVAL=...convenience variable (optional follow-up).Reviewer notes
Four review passes (2x GSD code-review, 2x project review prompt) applied; findings fixed down to nit level. CI runs build/lint/test on macOS only (cgo + CoreGraphics).