chore: upgrade to Go 1.26 and modernize - #6
Merged
Conversation
Move the go directive and CI to 1.26, then adopt the standard-library
features the code predated.
Go 1.26's only language change is new(expr), which has no site here: no
pointer-helper generics exist, and the &cloned returns in event.go mutate
the local first. errors.AsType likewise does not apply, since this module
uses errors.Is exclusively. The reductions therefore come from slices,
maps, cmp and the min/max builtins, which the code had not yet taken up.
- slices/maps replace hand-rolled clone, contains, equal, sort and
dedup loops
- slices.Sorted(maps.Keys(...)) replaces collect-then-sort for
deterministic map iteration
- min/max replace clamp blocks; Duration.Abs replaces a manual negate
- sync.WaitGroup.Go, range-over-int and strings.SplitSeq in tests
- a generic asEvents helper collapses six copies of the same
widen-to-interface loop in gesture and action
- ControlOf extracts the stick and trigger mapping it repeated four
times across its value and pointer cases
Two idioms are deliberately kept. The append([]byte(nil), x...) clones in
audit/recorder.go and merkle.go collapse empty to nil, which HMACKey
depends on: it is tested with != nil to select the HMAC chain, and
WithHMAC builds a non-nil empty key that is rejected later. And the
omitempty tags on nested struct fields, though genuine no-ops, are hashed
into the audit record chain, so changing them to omitzero would break
verification of logs already on disk.
Verified across darwin, linux and windows, with and without cgo, under
the race detector.
Guard tracked the operator's standing intent in three booleans — armed, latched and estop — mutated by hand in five places. The invariants between them were implicit: an emergency stop had to also force armed to false, and every assignment restated that by convention rather than by construction. Replace them with a compiled transition table from github.com/open-ships/statemachine. The lifecycle is now one field with three states (idle, armed, stopped) moved only by five commands: arm, disarm, trip, stop and reset. Arm's two refusals — no bound controller, and a latched emergency stop that only Reset may clear — become Guards on table rows rather than conditionals buried in the method. Mapping the booleans onto the table showed that latched was written in four places and read in none, so it is removed. The library holds no state, so the current lifecycle stays on the Guard under the Guard's own mutex, which already protects everything a transition touches. Arm still reports ErrUnbound and its own emergency-stop error rather than the machine's refusal. Fire wraps statemachine.ErrNotPermitted, and letting that sentinel into this package's public error contract would break callers comparing with ==. Guard.Evaluate's published State (safe, armed, live) is unchanged: it is derived per call from live conditions, which is a separate concept from the lifecycle this table models. Behavior was checked against a verbatim transcription of the replaced boolean logic over every command sequence up to depth six, bound and unbound, comparing Arm's error and both predicates Evaluate reads.
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.
Upgrades the module to Go 1.26 and takes up the standard-library features the code predated. Two commits, separable for review.
1.
chore: upgrade to Go 1.26 and modernizego.mod, CI and the README move to 1.26.I checked what 1.26 actually offers here by diffing the 1.25.12 and 1.26.5 source trees and reading the compiler's version gates, rather than working from memory. Two results worth recording:
new(expr)is 1.26's only language change and has no site in this module. No pointer-helper generics exist, and the&clonedreturns inevent.gomutate the local first.errors.AsTypedoes not apply — this module useserrors.Isexclusively and nevererrors.As.So the reductions come from
slices,maps,cmpand themin/maxbuiltins, which the code had not yet adopted:slices/mapsreplace hand-rolled clone, contains, equal, sort and dedup loopsslices.Sorted(maps.Keys(...))replaces collect-then-sort for deterministic map iterationmin/maxreplace clamp blocks;Duration.Absreplaces a manual negatesync.WaitGroup.Go, range-over-int andstrings.SplitSeqin testsasEventshelper collapses six copies of the same widen-to-interface loop ingestureandactionControlOfextracts the stick/trigger mapping it repeated four times across value and pointer cases2.
refactor: model the safety lifecycle as a state machineAdopts
github.com/open-ships/statemachineforsafety.Guard's operator lifecycle — the one place in the repo with a genuine event-driven state machine.Three booleans (
armed,latched,estop) mutated by hand in five places become one field with three states, moved by five commands through a compiled transition table.Arm's two refusals — no bound controller, and a latched e-stop that onlyResetmay clear — are nowGuards on table rows rather than conditionals inside the method.Mapping the booleans onto a table showed
latchedwas written in four places and read in none. It is removed.Armstill reportsErrUnboundand its own emergency-stop error rather than the machine's refusal, so the package's public error contract is unchanged for callers comparing with==.Deliberately not changed
omitemptyon nested struct fields (event.go,recorder.go) are genuine no-ops thatomitzerowould fix, but they are hashed into the audit record chain andcompat_test.gopins format versions 1–3. Changing them breaks verification of logs already on disk — that is a format-version bump, not a cleanup.append([]byte(nil), x...)inaudit/recorder.goandmerkle.gocollapses empty to nil, whichHMACKeydepends on: it is tested with!= nilto select the HMAC chain, andWithHMACbuilds a non-nil empty key rejected later.slices.Clonepreserves empty and would change chain selection.cloneEvent's type switch is the largest remaining block. Giving all nine event types aCloneEvent()method collapses it to ~6 lines, but value-receiver methods are in*T's method set, so pointer events would return as values — silently changing the dynamic type for external processors publishing*ButtonEvent. A generic helper preserves types but turns one itab-dispatched switch into a linear scan of nine closures, on a per-event per-subscriber dispatch path. The duplication is load-bearing.Verification
Both commits build and pass independently. Full matrix on the final tree: darwin/linux/windows (plus linux-arm64),
CGO_ENABLED0 and 1,go vetper platform,gofmt, and-race.modernizereports nothing outside theomitemptyitems above.Because the two behavior-sensitive rewrites had thin existing coverage, each was checked against a verbatim transcription of the code it replaced, then the check was deleted:
Arm's error and both predicatesEvaluatereadsControlOf, over all six arms including invalid IDs and nilNote
stickControl/triggerControlnow exist as identical unexported copies inteleop,gestureandaction. Unifying them means exporting new public API, so I left the copies — happy to do it if wanted.