Package structure, locking, error-handling & graceful context#368
Merged
Conversation
jenstopp
force-pushed
the
refactor/pkg-structure
branch
from
July 22, 2026 22:47
2806c76 to
168daa7
Compare
RiSKeD
approved these changes
Jul 23, 2026
RiSKeD
left a comment
RiSKeD
left a comment
Contributor
There was a problem hiding this comment.
LGTM, although this PR touches a lot of individual parts all at once.
Two problems surface once `go test -race` runs on every commit, which CI does: - The broker test double lazily created its unblock channel inside Receive on the worker goroutine while the test read and closed it — a data race the detector flags in TestBroker_CancelDuringBlockedReceive. Require the channel at construction so the worker only ever reads it, and close it unconditionally. - The error-path tests call session.Print from the test goroutine to trigger a worker send. Before the session gained a done-guard, Print blocks forever if the workers already tore down on the injected error, so the test deadlocks under the scheduling the race detector induces. Run those Print calls in a goroutine so a blocked send leaks harmlessly instead of wedging the test; the assertions already tolerate whether the send error surfaces. Fixing both here, before the package split, keeps every commit green under `go test -race`. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
AllModules yields every module across all devices/commands (iter.Seq) for whole-system sweeps such as agent init/deinit. Find returns the bare ErrDeviceNotFound for a single device, joining the CmdNames/FindCmd error+sentinel convention. The agent Lock handler now uses Find instead of raw device-map indexing, mirroring the Commands/Details errors.Is mapping. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Break up the grab-bag internal/dutagent package by concern: - internal/dutagent/locker: in-memory device reservation state (Locker; constructor renamed New, value types Info/Error/DeviceState to drop the package-name stutter). - internal/dutagent/session: the live module<->client I/O plane (Broker, Stream, and the unexported backend implementing module.Session). - Module lifecycle (Init/Deinit) folds into the dutagent main as initModules/deinitModules, iterating via dut.Devlist.AllModules and logging through the request-context logger; runModule stays in states.go. internal/dutagent is now a namespace directory with no package of its own. The locker/session extraction plus lifecycle fold is a deliberate asymmetry: cohesive, independently-tested state/IO become packages, while single-consumer startup glue lives with its only caller. Adds a lifecycle test (error aggregation + run-all-on-error) and retargets the forbidigo setup-file exception to the new package paths. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Split internal/buildinfo's mixed concerns into focused packages: - internal/compat: the semver compatibility policy (Compare -> Result with Verdict, Field, Cmp, Valid). It owns the tolerate/reject thresholds but holds no message wording and no app terms; Field is a structural label callers turn into text (never a threshold — the policy is non-monotonic). - internal/rpc: the connect wrapper — the two version interceptors, renamed by policy to NewVersionEnforcer (agent, rejects) and NewVersionAdvisor (client, warns) with a local reason() wording helper; typed h2c client constructors NewDeviceClient/NewRelayClient; an h2c ListenAndServe hiding the *http.Server; the Run bidi-stream adapter RunStream; and the VersionHeader const. This removes the newInsecureClient/spawnClient duplication across all three mains and the ad-hoc streamAdapter in dutagent, and moves the version verdict's English prose out of the policy layer into rpc. dutserver's relay guard now checks the structural compat.Field == Major directly instead of laundering it through a verdict. internal/buildinfo is left as pure build metadata. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Concentrate connect status-code conversion in the RPC handler layer and keep internal packages returning plain wrapped errors and sentinels. - dutserver: replace the version-guard panic with a CodeFailedPrecondition error and drop the ineffective top-level recover - dutctl: return after dispatching lock/unlock instead of falling through, and translate connect codes into friendly messages - dutagent: map device/command not-found to CodeNotFound and use a single cancellation code (Canceled/DeadlineExceeded) across the FSM boundary - session: add ErrBadFileTransfer and stop the per-worker triple-log - output: drop the dead Flush error return (log-and-drop on failure) - dut: add ErrUndefinedArgReference; chanio: skip empty messages in Read - document sentinel, connect-code and panic contracts across these packages Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Doc-comment pass over the module and support packages against the Google Go doc-comment conventions: - remove process/planning narration from comments and tests, describing only durable behavior - fix godoc rendering: indent verbatim lists and replace markdown backticks with doc links or plain text - add missing exported doc comments and correct stale API references - document non-obvious behavior (argument contracts, non-interruptible waits, cleanup obligations) while leaving trivial interface methods undocumented Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Align Deinit with the log-or-return convention: it logged the session-close error at debug and also returned it, but deinitModules already aggregates that error with the module's device and command identity, so the debug line only duplicated it with less context. Drop the log and return the error. Rename the withSession helper to withReconnect. "session" is overloaded in dutctl (module.Session, the internal/dutagent/session package), so withSession read as if it dealt with those; the helper runs a command and, if the connection is stale, reconnects and retries once, so name it for that behavior. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The HTTP headers that carry RPC metadata alongside the protobuf messages had no public home: the version header was defined in internal/rpc and the user header in pkg/lock, neither reachable by an external client implementation. Introduce a public pkg/headers holding both names: User, the caller identity carried in the standard From header, and Version, used for the build-compatibility handshake. Rename the version header from X-Dutctl-Version to Dutctl-Version, dropping the X- prefix deprecated by RFC 6648. The version interceptors and the relay adopt pkg/headers, and the client stamps requests with headers.User. BREAKING CHANGE: the version header is renamed from X-Dutctl-Version to Dutctl-Version and moved from internal/rpc to the new public pkg/headers. External clients must send and read the new name. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The Lock, Unlock and Run handlers read the caller identity from an HTTP header inline, coupling them to that transport. Introduce internal/auth with an Identity type carried on the request context: a connect interceptor resolves the identity once from the headers.User request header, and handlers read it back with auth.FromContext. The interceptor is the single place the identity source is wired in, so an authenticated source such as an mTLS peer certificate can replace it later without touching the handlers. Identity is a struct so a verified principal can be added the same way. This removes the pkg/lock helpers for the user header and the default user, which pkg/headers and internal/auth now supersede. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The Run RPC guarded a command's device with an auto-lock released by a non-deferred call after fsm.Run returned. A panic in a state function unwinds past that call (fsm.Run does not recover), so the auto-lock, which never expires, leaked and left the device busy for other users until an agent restart. Record the acquired lock on a pointer holder that acquireAutoLock writes and the Run handler reads, and release it from a single deferred cleanup that runs on normal, error and panic exits. This replaces both the non-deferred safety net and the separate releaseAutoLock state, collapsing the release into one path. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Locks are advisory and have no maximum by design: any caller may force-release another's lock, so fair sharing is cooperative rather than enforced. Make that posture explicit and usable end to end. A Lock request with duration_seconds == 0 now means "unset": the agent applies its default (30m) rather than rejecting it, so a client can omit the duration and defer the policy to the agent. A negative duration is still rejected. The dutctl client sends 0 when no duration is given and warns when an explicit duration is unusually long, nudging callers to release devices when done. Document the model where callers look: the LockRequest proto contract and the client usage text. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
List reported only the explicit lock slot, so a device busy with a running command (auto-locked, no explicit lock) appeared free. A caller would try it and hit an unexpected "locked by ..." rejection. Report whichever slot holds the device, with the explicit slot shadowing the auto slot when both are held. An auto-lock has no time-based expiry, so it surfaces with expires_at 0; the text formatter now renders any no-expiry lock as "in use by <owner>" rather than "locked by", distinguishing a running command from a timed reservation without a new proto field. Add an expiresAtUnix helper shared by List and Lock to map the zero time to 0 instead of a year-1 timestamp. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The reserved-command config error (lock/unlock) now carries the YAML line, so it points at the offending command like the module-level errors do. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
All output formatters now default a nil Stdout/Stderr to os.Std* via a shared withDefaultWriters helper. Previously only the text formatter did, so the json, yaml and oneline formatters would panic on a nil writer. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
locker.StatusAll now copies each stored Info as a whole struct instead of rebuilding it field by field, so a newly added field can no longer be dropped from the snapshot silently. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Enable the race detector in CI (go test -race) so concurrency regressions in the locker, broker and session are caught automatically. It immediately flagged a data race in the broker test double: the worker goroutine lazily created the testStream unblock channel while the test read and closed it. Initialize the channel up front so the worker only ever reads it. Also add a StatusAll pruning test: an expired explicit lock must drop out of the snapshot, covering the List visibility path alongside the existing expiry test. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
A header-less caller is assigned a fresh anonymous identity on every request, so one that took a lock could never present the same identity again to release it — leaving a stuck lock that blocks others until it expires. Reject an anonymous caller with CodeUnauthenticated on Lock and on a normal Unlock. caller() now returns the full Identity so handlers can see IsAnonymous; a requireNamed helper enforces the rule. Run is unaffected — its auto-lock is acquired and released within one request — and a forced Unlock stays open as the cooperative override, so anyone may still break a stuck lock. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Explicit acquire and release are already logged by the handlers, but a lock that ends by expiry is pruned lazily inside the locker with no record — a reservation silently hands off to the next user. Log that event at Info where it is pruned, and trace auto-lock acquire/release at Debug for contention debugging. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Each of the four formatters re-derived the same "stderr on error, else stdout" pick for non-buffered output. Extract a single streamFor helper and route all four through it. Buffered output stays per-formatter, as the buffer shapes (bytes.Buffer, strings.Builder, struct slices) differ and share no common selection. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The oneline/csv formatter rendered any held device as "name=locked:owner", so an auto-lock guarding a running command was indistinguishable from an explicit reservation. Encode the two states separately as "name=in-use:owner" and "name=locked:owner", mirroring the text formatter. Also document the line-format contract, which previously lived only in the code. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The client silently swallowed extra positional arguments: "unlock" ignored everything after it, so the natural "dutctl <dev> unlock --force" left force unset (Go's flag parser treats it as a positional, not the -force flag), and "lock" ignored anything past the duration. Tighten dispatch so lock takes at most one duration, unlock takes only the optional "force" keyword, and the help keyword must stand alone; anything else is a command-line error that prints the usage synopsis. Force-unlock moves from the global -force flag to a positional keyword, matching the other bare keywords (lock, unlock, help) and removing the flag-ordering trap. BREAKING CHANGE: the -force flag is removed; use "dutctl <device> unlock force" to break a lock held by another user. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The locker's reporting vocabulary was generic and the StatusAll snapshot exposed two independent slot pointers that its only consumer immediately collapsed. Rename Info to Hold and replace the Slot type (ExplicitSlot/ AutoSlot) with a Kind (Reserved/Busy) that names the device's state rather than its storage mechanism. Collapse StatusAll to map[string]Hold, returning the effective holder per device (a live reservation shadows a concurrent Busy hold), which removes DeviceState and moves the precedence rule out of the List handler. Internal maps and the expiry/force-clear logs adopt the same reserved/busy wording. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
dutctl reserves a handful of command-line keywords, but the knowledge of which names those are lived only in a hardcoded lock/unlock check in config validation and in scattered string literals across the client dispatch and the agent. Introduce internal/keyword as the single source of truth: the keyword constants plus a position-scoped reservation policy, referenced by config validation, the client dispatch, and the agent's Details check alike. Reservation is scoped by grammar position so it restricts naming no more than necessary. A device named list or version is unreachable (the keyword is dispatched instead of addressing the device), so config validation now rejects those device names — closing a latent gap where such a device silently could not be targeted. Command names lock and unlock are rejected as before, plus help, which keeps "dutctl <device> help" unambiguous. Names outside their colliding position stay free: a device may be named lock, a command list. ErrReservedCommand is replaced by keyword.ErrReservedName. The agent's unknown-keyword message now quotes the accepted value via %q. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Register panicked on the module IDs "help" and "info", a reservation carried unchanged from the original plug-in system. It guards against nothing: a module ID lives in its own namespace — it appears only as the config "module:" field and is resolved through module.New — so it never occupies a device, command, or argument position on the command line and cannot collide with a CLI keyword. "info" corresponds to no keyword at all. Remove the check so module authors are not restricted without cause; genuine command-line reservations are enforced by internal/keyword. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
LockResponse repeated LockInfo's owner/locked_at/expires_at fields verbatim. Rename LockInfo to LockState (shedding the generic "Info" suffix, echoing the locker.Hold rename) and embed it in both DeviceInfo and LockResponse, so the lock-state shape has a single definition. Also rename DetailsRequest.cmd to command for consistency with Command.command. Regenerated the Go bindings and updated the agent handlers, the client, and their tests. Verified end-to-end with a real client/agent lock round-trip. BREAKING CHANGE: LockResponse now carries its lock fields under a nested LockState message, and DetailsRequest.cmd is renamed to command. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The dutctl client ran every RPC on a bare context.Background(): no deadline, and Ctrl-C killed the process mid-call. dispatch now creates one signal.NotifyContext (SIGINT/SIGTERM) and threads it into every RPC. The five unary calls wrap it in a 30s per-call timeout; connect encodes that as a grpc-timeout header, so the agent handler inherits the same deadline. The streaming Run keeps the shared signal context but no overall deadline. A signal-cancelled RPC maps to the conventional interrupted status (exit 130) via asInterrupt, so unary Ctrl-C matches Run instead of surfacing a raw CodeCanceled as a generic failure. The shared h2c transport gains a 10s DialContext bound: stream-safe (it caps only connection setup) and the only dial bound for the deadline-less Run. Client.Timeout and ResponseHeaderTimeout stay unset on purpose — the client is shared with Run, whose server writes response headers lazily, so either would abort a slow stream. IdleConnTimeout (daemon-side pool hygiene) is deferred to the dutserver work. The agent's one-shot registration RPC gains a 10s deadline so an unreachable server fails fast instead of hanging startup. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Both daemons handled SIGINT/SIGTERM out of band: a goroutine ran cleanup while the RPC service blocked forever in ListenAndServe, so a signal cut in-flight RPCs and (dutagent) ran module Deinit on an unbounded context. rpc.ListenAndServe now takes a context. It serves in a goroutine and, when the context is cancelled, calls http.Server.Shutdown to drain in-flight requests (bounded by a 15s grace period) before returning. The shutdown context is derived with context.WithoutCancel so it keeps the request's values but not the already fired cancellation, which would otherwise make Shutdown return immediately and skip the drain. A serve failure such as a bind error is returned; callers classify a graceful stop via ctx.Err(). The listener/serve split is factored out (unexported serve) so a test can drive a real in-flight request against a known address. Both daemons replace watchInterrupt with signal.NotifyContext threaded into the service, and classify the outcome: a cancelled context shuts down cleanly (exit 0), otherwise the server failed to serve (exit 1). dutagent bounds module Deinit with a 15s timeout during shutdown, so a wedged module cannot hang teardown. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The module-facing session methods (Print*, Console's stdin/stdout/stderr, RequestFile,
SendFile) run on the module goroutine, carry no context, and block on unbuffered channels
whose peer is a broker worker. Normally the module finishes before the workers stop, so this
never bites. But when a worker exits first — a client disconnect or transport error, or a
malformed file message — the two workers tear each other down while the module runs on: its
context is a sibling of the workers', so worker cancellation never reaches it. The module's
next session call then blocks forever, leaking the goroutine for the process lifetime.
Freeze the workers' done signal onto the session alongside the logger and have every
module-facing channel op select on it: Print* drop, RequestFile/SendFile return an error, and
chanio's ChanReader/ChanWriter — now taking an optional done channel — return io.EOF and
io.ErrClosedPipe. A nil done keeps the old uncancellable behaviour, so the transient per-file
readers are unaffected. This unwinds a module parked in a session call, including one in
io.ReadAll or bufio on stdin. A module parked in its own non-session code (a subprocess, a
socket, sleep) is beyond the reach of any signal and still needs cooperative context handling;
that is left for later.
Also close two teardown gaps in the worker:
- The receive-loop goroutine leaked permanently on cancel: after stream.Receive returned it
sent on resCh, which the returned main loop no longer drains, blocking forever. Guard the
send with ctx.Done so it exits, and correct the comment that claimed this could not happen.
- The client-to-module file hand-off had no ctx escape, so an abandoned transfer could wedge
the worker and, through wg.Wait, the broker. Guard it with ctx.Done.
Set currentFile before sending the file request rather than after, so a fast client response
cannot be validated against a name that is not yet recorded.
Add mutation-verified regression tests for the module-unblock and receive-loop-exit paths,
plus chanio done tests.
Signed-off-by: Jens Topp <jens.topp@blindspot.software>
The session change unwinds a module blocked in a session call; this closes the other half — a module blocked in its own work — plus the remaining startup and transport context gaps. A module that returns ctx.Err() already maps to CodeCanceled at the RPC boundary, so this is per-module work with no plumbing changes. Subprocess modules (shell, agent-status, flash-emulate, flash) now run their tool via a new internal/procexec helper instead of a bare exec.Command. The helper puts the command in its own process group and, on cancellation, signals the whole group so forked children die too — a plain exec.CommandContext signals only the direct child, orphaning grandchildren and, because they inherit the output pipes, blocking Wait until they exit. shell/agent-status/flash-emulate stop with SIGKILL; flash stops with SIGTERM and a grace period so a flashing tool is asked to finish cleanly rather than cut mid-write. This drops all four //nolint:noctx directives. The time-wait module now selects on ctx while waiting instead of an uninterruptible time.Sleep, and ssh dials with a context-aware net.Dialer and closes the client on cancellation so a blocked handshake or command run unwinds. Startup and transport: module initialization derives from the signal context and is bounded by a timeout, so a wedged Init is both interruptible and fails startup instead of hanging forever. The shared HTTP client sets IdleConnTimeout to reap stale pooled connections (pool hygiene for the long-lived relay). The relay binds its upstream stream to a WithCancelCause context so a forwarding goroutine exiting tears the upstream down promptly, with a cause recorded for the completion log. Add a mutation-verified test that procexec.Command kills the whole process group. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
currentFile is read and written from three goroutines — the module goroutine (SendFile) and both broker workers — with no channel handing it between them: their ordering runs through the client round-trip, which is not a Go happens-before edge. That is a data race on the two-word string header, where a torn read could crash. Add a mutex guarding the field, reached through currentFileName/setCurrentFile accessors, and hold the lock only around the field access, never across a channel operation or stream Send. The two multi-read sites read once into a local so the value stays consistent within the block. Add a race test that reads and writes currentFile concurrently; run under -race it fails without the mutex. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
jenstopp
force-pushed
the
refactor/pkg-structure
branch
from
July 24, 2026 07:05
168daa7 to
442cb9b
Compare
Closed
This was referenced Jul 24, 2026
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
2 participants
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.
A consolidation branch. It reorganizes the internal
packages, hardens the locking/access model and error handling, cleans up the output
formatters, and makes the whole stack honor context cancellation.
Please review commit-by-commit
The branch is large but every commit is self-contained and builds independently.
Each commit message carries the why - the problem, the reasoning, and the trade-offs -
so the intended review path is commit by commit, reading each message alongside its
diff, rather than the squashed range. Breaking changes are marked
!and repeated underBreaking changes below.
The first commit is test infrastructure that keeps the rest green under the race detector:
d107aa0test: harden broker tests for the race detector — fixes a pre-existing data racein the broker test double and de-flakes the error-path tests, so every later commit passes
go test -race.The remaining commits cluster into six themes:
A. Package structure & boundaries
4c21e37refactor: addDevlist.AllModulesiterator andFindaccessorbd37049refactor: splitinternal/dutagentintolockerandsessionpackagesc4846ecrefactor: extract compat policy and rpc wrapper frombuildinfo(newinternal/compat,internal/rpc)d72cea7fix!: publish the rpc header contract aspkg/headers53b46cbrefactor: resolve caller identity through aninternal/authseamB. Error handling, doc-comments & ipmi
9d8bbcffix: harden error handling and connect status-code mappinga0a3f98docs: align doc comments with the Go style guide2639190refactor: clean up the ipmi moduleC. Locking model & access control
11bd817fix: require a named caller to lock or release a device70ac84dfix: release the Run auto-lock on panic and error pathsab7056ffeat: apply an agent default lock duration and nudge cooperative use029d8abfix: surface auto-locks in List so a busy device is not shown as free19b404cfeat: log lock expiry and auto-lock lifecycle at the locker8a5a335refactor: model the locker around Holds with a Kind, not slots03fe095refactor: copylocker.StatusAllentries as whole structs4605b08test: run the suite under-raceand fix the race it surfaced569ce3crefactor!: embedLockStateinLockResponseand align proto namingD. Reserved keywords, CLI args & config
0b95e04feat: consolidate reserved keywords intointernal/keywordcb15d8frefactor: drop the vestigial reserved module-ID check170402ffix: carry the YAML line in reserved-command config errorsd864fd0fix!: makeforcean unlock keyword and reject malformed lock/unlock argsE. Output formatters
a20ace7fix: default nil formatter writers toos.Stdout/os.Stderr3ca35a2refactor: unify immediate writer selection across output formatters732d207fix: distinguish in-use from locked in oneline outputF. Graceful context handling
e4c2af2feat: bound client RPCs with a signal-aware per-call deadline8d35af6feat: drain in-flight RPCs on signal instead of cutting thema17c7b7fix: unblock module session I/O when its workers exit first8ae5806feat: honor context cancellation in modules, startup, and relay168daa7fix: guard sessioncurrentFilewith a mutexTogether, theme F makes cancellation unwind cleanly at every layer — client Ctrl-C, per-RPC
deadline, server drain, a module blocked in a session call (
a17c7b7), and a moduleblocked in its own work — subprocess, sleep, ssh (
8ae5806).Breaking changes
Three commits change the wire/CLI contract (relevant to the
2.0.0-alpharelease):d72cea7— version header renamedX-Dutctl-Version→Dutctl-Versionand moved topkg/headers.569ce3c—LockResponsecarries its lock fields under a nestedLockState;DetailsRequest.cmd→command.d864fd0— the-forceflag is removed; usedutctl <device> unlock force.Relationship to open issues & PRs
Cross-referenced against all open PRs and issues by file-overlap and intent.
Supersedes — can be closed once this merges (same goal, implemented here more completely):
a17c7b7(+168daa7); same fix, broader.0b95e04/cb15d8f; same registry asinternal/keyword.0b95e04/170402f; the branch's keyword-scoped config validation subsumes its narrow reserved-device check.Fixes:
0b95e04,cb15d8f,d864fd0.Partially addresses (moves the needle, doesn't fully close):
9d8bbcf), Unify queries to device list #85 unify device-list queries (4c21e37), Resolve nolint-comments or adapt linter settings #134 reduce nolint comments (dropped the fournoctxin8ae5806), Handle module writings to stderr clearly #200 module stderr handling (3ca35a2/a20ace7).Adjacent / touched in passing: issues #38, #113, #290.
(The closed issues #37/#42 "Use ctx gracefully" are the origin of theme F.)
Verification
build/vet/gofmt/go test -race ./.../golangci-lintin isolation — verified per commit (the exact per-commit CI command).receive-loop exit,
currentFilerace, process-group kill).agent survives; graceful shutdown drains in flight.