From b777ffcf58a5a4699da88d25fc8728cc6b4d7bcd Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Thu, 10 Sep 2026 09:18:54 +0000 Subject: [PATCH] chore: replace em dashes with hyphens, add no-em-dash rule to CLAUDE.md --- CLAUDE.md | 32 +++++++++++++++------------ internal/inventory/collect_windows.go | 2 +- internal/packages/packages.go | 2 +- internal/packages/parse.go | 6 ++--- internal/sync/packages.go | 6 ++--- internal/sync/sync.go | 12 +++++----- internal/winexec/encode.go | 4 ++-- internal/workloads/control_windows.go | 2 +- internal/workloads/docker.go | 6 ++--- internal/workloads/logs_windows.go | 6 ++--- internal/workloads/systemd_linux.go | 2 +- internal/workloads/units_other.go | 2 +- internal/workloads/winparse.go | 12 +++++----- internal/workloads/winparse_test.go | 4 ++-- internal/workloads/workloads.go | 2 +- 15 files changed, 52 insertions(+), 48 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9c2b042..7e49173 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,24 +30,24 @@ vantage-agent/ | Repository | Relationship | | ---------------- | ------------------------------------------------------------------------------------------------------ | | `vantage-shared` | a private Go module. `grpc/pb` and `grpc/codec` are the wire contract; `proto/` there documents them | -| `vantage` | the control plane. **No import in either direction** — the coupling is the gRPC wire, and it is entirely mediated by `vantage-shared` | +| `vantage` | the control plane. **No import in either direction** - the coupling is the gRPC wire, and it is entirely mediated by `vantage-shared` | **`vantage-shared` is private**, so every Go build needs `GOPRIVATE=gitea.hostxtra.co.uk/*` plus a credential. CI writes a netrc from `REGISTRY_USER` + `RELEASE_TOKEN` (**that token needs read access to the -`vantage` org**) — twice, because the `msi` job is Windows and Go looks for +`vantage` org**) - twice, because the `msi` job is Windows and Go looks for `_netrc` in the profile directory there, not `.netrc`. Locally, either a netrc or `git config --global url."git@gitea.hostxtra.co.uk:".insteadOf https://gitea.hostxtra.co.uk/`. ### A wire change is three steps, in order `shared/grpc/pb` is hand-written and shared by both sides, so a new message is a -compile error rather than a silent disagreement — but only once each side moves: +compile error rather than a silent disagreement - but only once each side moves: 1. release `vantage-shared` (and change `proto/vantage/v1/vantage.proto` in the same commit as the Go types) -2. bump the pin in `vantage`'s `server/go.mod` — live at the next push to main -3. bump the pin here — live only at the next `agent/v*` tag +2. bump the pin in `vantage`'s `server/go.mod` - live at the next push to main +3. bump the pin here - live only at the next `agent/v*` tag The control plane runs ahead of the fleet in between. That was true before the split too; it is now explicit in two `go.mod` files rather than implicit in a @@ -68,7 +68,7 @@ git tag agent/v1.2.0 && git push origin agent/v1.2.0 Builds `linux/amd64`, `linux/arm64` and `windows/amd64`, writes `checksums.txt`, creates the Gitea release. A second `msi` job on `windows-2022` builds the exe again, packages it with WiX and appends the MSI to the same release **through -the API** — `gitea-release-action` cannot find a tag with a slash in it. +the API** - `gitea-release-action` cannot find a tag with a slash in it. ### The self-update path, and what the move broke @@ -77,7 +77,7 @@ the API** — `gitea-release-action` cannot find a tag with a slash in it. SHA-256 from `checksums.txt` before swapping itself. That path is **compiled into the binary**. -**Every agent built before this move has the old path — `mrhid6/vantage` — +**Every agent built before this move has the old path - `mrhid6/vantage` - baked in, and releases are no longer published there.** For those agents the push-button update in the UI will fail: the download 404s. They are not stranded, because `/update` and `/update.ps1` are generated by the control plane @@ -85,8 +85,8 @@ at request time and point wherever the current server says, so re-running the update one-liner on a host moves it onto a build that knows the new address. After that, self-update works again permanently. -This was a deliberate choice — the alternative was publishing releases to a -repository that no longer holds the source — but it means **the fleet needs one +This was a deliberate choice - the alternative was publishing releases to a +repository that no longer holds the source - but it means **the fleet needs one pass of the update one-liner**, and the control plane must be redeployed with the new release paths *first*, or the one-liner points at the old repository too. @@ -96,7 +96,7 @@ too. - **It never reboots a host.** `ApplyUpdatesCmd` installs and stops there; `inventory.reboot_required` reports that one is owed. - **It decides what it will not touch.** The protected workload set is computed - and enforced agent-side — `vantage-agent.service`, `VantageAgent` on Windows, + and enforced agent-side - `vantage-agent.service`, `VantageAgent` on Windows, and its own container ID from `/proc/self/cgroup`. The control plane may name a target; the agent decides what it will do to itself. A server-side denylist alone would be bypassed by the next dispatch path someone adds, and the @@ -105,12 +105,12 @@ too. the control plane can name a port and nothing else. - **No `authorized_keys` management on Windows**, and no package inventory: a Windows agent never calls `ReportPackages`, so no `server_packages` document - exists for it at all — a different, earlier state than the `unsupported` a + exists for it at all - a different, earlier state than the `unsupported` a Linux distribution reaches when its family has no security feed. ## Platform split -Windows support is build tags, not runtime branches — `systemd_linux.go` / +Windows support is build tags, not runtime branches - `systemd_linux.go` / `services_windows.go` and the matching `control_` and `logs_` pairs. Windows collection runs PowerShell through `internal/winexec`, and **every script that reports data emits JSON that a build-tag-free parser reads**, so those parsers @@ -126,8 +126,8 @@ article ID: a Windows update is not a version bump of a named package. ## Two constants that mirror the control plane -Neither can be shared — this is a separate module and the control plane's are -under `internal/` — so both must change in step, by hand: +Neither can be shared - this is a separate module and the control plane's are +under `internal/` - so both must change in step, by hand: - the workload log cap, **500 lines and 256KB whichever binds first**, mirrored in the control plane's `services.MaxWorkloadLogLines` @@ -135,3 +135,7 @@ under `internal/` — so both must change in step, by hand: control plane's 20s `PingCmd`. The watchdog arms only **after** a first ping has been seen, so an older server that sends none is treated as working rather than put into a reconnect loop. + +## Writing style + +Never use em dashes (the long dash character) anywhere: code, comments, UI copy, docs, commit messages. Use a plain hyphen ` - `, a comma, a colon, or split the sentence instead. diff --git a/internal/inventory/collect_windows.go b/internal/inventory/collect_windows.go index 5226592..5dea7ac 100644 --- a/internal/inventory/collect_windows.go +++ b/internal/inventory/collect_windows.go @@ -28,7 +28,7 @@ func collect(r *pb.InventoryReport, includeStatic bool) { if m.TotalPhys > m.AvailPhys { r.Memory.UsedBytes = m.TotalPhys - m.AvailPhys } - // TotalPageFile is the commit limit — physical memory plus the pagefile — + // TotalPageFile is the commit limit - physical memory plus the pagefile - // so the pagefile alone is the difference. swapTotal := sub(m.TotalPageFile, m.TotalPhys) swapUsed := sub(sub(m.TotalPageFile, m.AvailPageFile), sub(m.TotalPhys, m.AvailPhys)) diff --git a/internal/packages/packages.go b/internal/packages/packages.go index a503223..5591709 100644 --- a/internal/packages/packages.go +++ b/internal/packages/packages.go @@ -16,7 +16,7 @@ const collectTimeout = 2 * time.Minute // // The format strings below are raw string literals on purpose. The "\t" and // "\n" reach dpkg-query and rpm as two characters each, and those tools do the -// interpreting themselves — Go must not consume the escapes first. +// interpreting themselves - Go must not consume the escapes first. func Collect() (OSRelease, []Package, error) { if runtime.GOOS != "linux" { return OSRelease{}, nil, fmt.Errorf("package collection is linux-only, got %s", runtime.GOOS) diff --git a/internal/packages/parse.go b/internal/packages/parse.go index 2365332..4e9e254 100644 --- a/internal/packages/parse.go +++ b/internal/packages/parse.go @@ -9,7 +9,7 @@ import ( ) // Package is one installed package as the distribution reports it. Version is -// the distribution's own version string, verbatim — never normalised, because +// the distribution's own version string, verbatim - never normalised, because // the advisory feeds are keyed on exactly this form. type Package struct { Name string @@ -29,7 +29,7 @@ type Package struct { // // The fifth column is why "rc" packages do not appear. dpkg-query -W lists // every package dpkg knows about, including ones removed with their config -// files left behind — a host that has upgraded its kernel a dozen times reports +// files left behind - a host that has upgraded its kernel a dozen times reports // a dozen old linux-modules versions that are not on disk, and the oldest of // them sorts first and reads as the installed version. Only "installed" is // installed. An empty status means dpkg did not understand the field, in which @@ -143,7 +143,7 @@ func splitAPK(s string) (name, version string) { // // It sorts first: the ordering of dpkg or rpm output is not guaranteed stable, // and an ordering-sensitive hash would resend the full ~150KB list every hour -// for no reason — a cost visible only as traffic. +// for no reason - a cost visible only as traffic. func Hash(pkgs []Package) string { lines := make([]string, 0, len(pkgs)) for _, p := range pkgs { diff --git a/internal/sync/packages.go b/internal/sync/packages.go index d50945d..eca0e49 100644 --- a/internal/sync/packages.go +++ b/internal/sync/packages.go @@ -15,7 +15,7 @@ import ( ) // collectPackagesFlag is written by the 30s key poll and read by the hourly -// package loop — two different goroutines, hence the atomic. +// package loop - two different goroutines, hence the atomic. // // It defaults to false, so an agent that has not yet completed a poll, or is // talking to a server too old to send the field, collects nothing. Off is the @@ -27,7 +27,7 @@ var collectPackagesFlag atomic.Bool // // Without it the boot-time package report loses a race it can only lose: the // hourly loop starts before the first poll, reads a flag that is still false by -// construction, and skips — so a freshly installed agent reports no packages for +// construction, and skips - so a freshly installed agent reports no packages for // an hour and the server shows nothing to scan. // How long the boot package report waits for that first poll. Two poll // intervals plus slack: long enough to cover one failed attempt, short enough @@ -43,7 +43,7 @@ func markFirstPoll() { firstPollOnce.Do(func() { close(firstPoll) }) } // waitFirstPoll blocks until the flag is known, or gives up. The wait is // bounded because this loop also reports OS updates, which do not depend on the -// flag at all — a control plane that cannot be polled must not silence those too. +// flag at all - a control plane that cannot be polled must not silence those too. func waitFirstPoll(ctx context.Context, limit time.Duration) { t := time.NewTimer(limit) defer t.Stop() diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 0890f7f..1e49c56 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -134,7 +134,7 @@ func poll(client *grpcclient.Client, cfg *config.Config, version string) error { const streamHealthyAfter = time.Minute // Stream staleness. The server beats every 20s, so 70s tolerates three missed -// beats before the stream is written off — high enough that a slow network or a +// beats before the stream is written off - high enough that a slow network or a // briefly busy server does not cost a reconnect, low enough that an agent is // not uncommandable for minutes after a control-plane restart. const ( @@ -142,7 +142,7 @@ const ( streamStaleCheck = 10 * time.Second // How often a healthy stream reports itself. Also the interval at which an - // agent talking to a control plane too old to send heartbeats says so — + // agent talking to a control plane too old to send heartbeats says so - // that agent is running without a watchdog, and the journal should not be // silent about it. pingSummaryInterval = 5 * time.Minute @@ -184,7 +184,7 @@ func runCommandStream(ctx context.Context, cfg *config.Config) { // The uptime is in the line because it is what distinguishes a stream // that never worked from one that ran for hours and was dropped by a - // deploy — and it is the same measure that decides whether the backoff + // deploy - and it is the same measure that decides whether the backoff // resets, so a reader can see why the delay is what it is. up := time.Since(started).Truncate(time.Second) if err != nil { @@ -253,7 +253,7 @@ func connectAndHandleStream(ctx context.Context, cfg *config.Config) error { // the operator watches nothing happen. // // The watchdog only arms once a ping has actually been seen. A server too - // old to send them must not be treated as dead — that would put the agent + // old to send them must not be treated as dead - that would put the agent // in a reconnect loop against a control plane that is working perfectly. var ( lastMu sync.Mutex @@ -283,7 +283,7 @@ func connectAndHandleStream(ctx context.Context, cfg *config.Config) error { // Reported periodically rather than per beat: at one every 20s the // journal would be nothing else. The count is what makes a partial - // failure visible — beats arriving but fewer than expected is a + // failure visible - beats arriving but fewer than expected is a // different problem from beats stopping altogether. summary := time.NewTicker(pingSummaryInterval) defer summary.Stop() @@ -450,7 +450,7 @@ func runInventory(ctx context.Context, cfg *config.Config) { r := inventory.Collect(static) r.ServerId = cfg.ServerID r.AgentToken = cfg.AgentToken - // Static snapshots only — every 15 minutes, not every 30 seconds. On + // Static snapshots only - every 15 minutes, not every 30 seconds. On // Windows this spawns a PowerShell process, which is not something to // do twice a minute forever, and a host rebooted by hand clearing the // flag within a quarter of an hour is soon enough. diff --git a/internal/winexec/encode.go b/internal/winexec/encode.go index 8ea7cc8..9dc72e4 100644 --- a/internal/winexec/encode.go +++ b/internal/winexec/encode.go @@ -1,7 +1,7 @@ // Package winexec runs PowerShell on Windows hosts. // -// It exists because three subsystems — updates, workload collection and -// workload logs — all need the same invocation, and because getting a +// It exists because three subsystems - updates, workload collection and +// workload logs - all need the same invocation, and because getting a // multi-line script past Go quoting, cmd.exe quoting and PowerShell's own // parser is a problem worth solving once. package winexec diff --git a/internal/workloads/control_windows.go b/internal/workloads/control_windows.go index 2f3dcaf..3049287 100644 --- a/internal/workloads/control_windows.go +++ b/internal/workloads/control_windows.go @@ -9,7 +9,7 @@ import ( "gitea.hostxtra.co.uk/vantage/vantage-agent/internal/winexec" ) -// AgentUnit is the service this agent runs as — the NSSM service name written +// AgentUnit is the service this agent runs as - the NSSM service name written // by installer/setup.ps1. Change one, change the other. const AgentUnit = "VantageAgent" diff --git a/internal/workloads/docker.go b/internal/workloads/docker.go index 8584154..2e8a050 100644 --- a/internal/workloads/docker.go +++ b/internal/workloads/docker.go @@ -30,8 +30,8 @@ const dockerTimeout = 30 * time.Second // dockerInspect is the subset of `docker inspect` output we read. // // We use inspect rather than `docker ps --format '{{json .}}'` because ps -// reports health and uptime inside a human Status string — "Up 2 hours -// (healthy)" — and anything built on that is parsing English that is +// reports health and uptime inside a human Status string - "Up 2 hours +// (healthy)" - and anything built on that is parsing English that is // localised, reworded between releases, and silently different for a paused or // restarting container. inspect gives typed fields instead. type dockerInspect struct { @@ -59,7 +59,7 @@ type dockerInspect struct { } // collectDocker enumerates containers. It returns ok=false with an empty error -// string when Docker is simply not installed — the common case on this fleet, +// string when Docker is simply not installed - the common case on this fleet, // and not a fault. func collectDocker(ctx context.Context) ([]Workload, bool, string) { if _, err := exec.LookPath("docker"); err != nil { diff --git a/internal/workloads/logs_windows.go b/internal/workloads/logs_windows.go index f55fe0e..1eaabed 100644 --- a/internal/workloads/logs_windows.go +++ b/internal/workloads/logs_windows.go @@ -25,7 +25,7 @@ func logsPlatform(ctx context.Context, kind, id string, tail int) (string, error // Timestamps are formatted PowerShell-side rather than left to // ConvertTo-Json, whose DateTime rendering differs between PowerShell - // versions — one of them emits /Date(1699...)/. + // versions - one of them emits /Date(1699...)/. // // $ErrorActionPreference = 'SilentlyContinue' because Get-WinEvent // treats "no events matched" as a terminating error, and a quiet @@ -38,7 +38,7 @@ func logsPlatform(ctx context.Context, kind, id string, tail int) (string, error // ProviderName includes the host-wide Service Control Manager, so a // -MaxEvents cap of exactly tail would apply to the combined stream - // before parseEvents narrows SCM rows down to this service — on a + // before parseEvents narrows SCM rows down to this service - on a // host with busy service churn the target's own events could be // squeezed out of the window entirely. Over-fetch instead, hard-capped // so a pathological host cannot pull an unbounded batch across the @@ -76,7 +76,7 @@ ConvertTo-Json -InputObject @($rows) -Depth 3 -Compress } // serviceDisplayName resolves a service's display name, which is what Service -// Control Manager events name it by. An empty answer is fine — the filter then +// Control Manager events name it by. An empty answer is fine - the filter then // matches on the service name alone. func serviceDisplayName(ctx context.Context, id string) string { out, err := winexec.Run(ctx, diff --git a/internal/workloads/systemd_linux.go b/internal/workloads/systemd_linux.go index 49a4c58..f81201b 100644 --- a/internal/workloads/systemd_linux.go +++ b/internal/workloads/systemd_linux.go @@ -15,7 +15,7 @@ const systemdTimeout = 30 * time.Second var excludedPrefixes = []string{"systemd-", "user@", "user-", "session-", "init.scope"} // collectUnits enumerates services in two passes, because "running or -// failed" and "enabled but stopped" are different questions — and an enabled +// failed" and "enabled but stopped" are different questions - and an enabled // unit that is not running is exactly the one worth seeing. func collectUnits(ctx context.Context) ([]Workload, bool, string) { if _, err := exec.LookPath("systemctl"); err != nil { diff --git a/internal/workloads/units_other.go b/internal/workloads/units_other.go index 3d15b08..7ae8e46 100644 --- a/internal/workloads/units_other.go +++ b/internal/workloads/units_other.go @@ -1,6 +1,6 @@ //go:build !linux && !windows -// The build constraint is load-bearing — see updates_other.go. +// The build constraint is load-bearing - see updates_other.go. package workloads import ( diff --git a/internal/workloads/winparse.go b/internal/workloads/winparse.go index 5aaa20b..8a70576 100644 --- a/internal/workloads/winparse.go +++ b/internal/workloads/winparse.go @@ -19,7 +19,7 @@ type winService struct { } // exitCodeNeverStarted is ERROR_SERVICE_NEVER_STARTED. A stopped service -// carrying it has not failed — it has not run since boot — and painting that +// carrying it has not failed - it has not run since boot - and painting that // red would cry wolf on every host. const exitCodeNeverStarted = 1077 @@ -52,8 +52,8 @@ func servicePath(pathName string) string { } // exeBoundaryIndex finds the first ".exe" (case-insensitive) in s that -// actually ends the executable name — followed by end-of-string, whitespace, -// or a double quote — rather than continuing into a longer segment such as +// actually ends the executable name - followed by end-of-string, whitespace, +// or a double quote - rather than continuing into a longer segment such as // ".exec". It returns -1 when no such occurrence exists, so a path like // `C:\Program Files\Ad.exec\tool.com -flag` is not misparsed by matching the // ".exe" inside "Ad.exec" and silently dropping the real filename. @@ -113,8 +113,8 @@ func parseServices(jsonText, systemRoot string) ([]Workload, error) { continue } - // The wire shape is shared with the systemd collector — both report - // under kind "unit" — so the state word has to be too, or the UI + // The wire shape is shared with the systemd collector - both report + // under kind "unit" - so the state word has to be too, or the UI // (which colours and filters on it, and does so before it knows // which platform sent the row) needs two vocabularies for one kind. // running/stopped/failed become active/inactive/failed to match. @@ -168,7 +168,7 @@ type winEvent struct { // applied before SCM rows are narrowed down to this service would squeeze the // target's own events out of the window on a host with busy service churn. // tail is therefore applied here, AFTER filtering and AFTER the oldest-first -// reversal, keeping the last tail lines — the most recent lines are the ones +// reversal, keeping the last tail lines - the most recent lines are the ones // worth keeping, matching capLog's front-trim reasoning in the shared // logs.go. func parseEvents(jsonText, serviceName, displayName string, tail int) (string, error) { diff --git a/internal/workloads/winparse_test.go b/internal/workloads/winparse_test.go index 9106bea..f080688 100644 --- a/internal/workloads/winparse_test.go +++ b/internal/workloads/winparse_test.go @@ -13,7 +13,7 @@ func TestServicePath(t *testing.T) { {`"C:\no\args.exe"`, `C:\no\args.exe`}, {``, ``}, // ".exe" appearing inside an earlier segment ("Ad.exec") must not be - // treated as the end of the executable — that would drop the real + // treated as the end of the executable - that would drop the real // filename and arguments. {`C:\Program Files\Ad.exec\tool.com -flag`, `C:\Program`}, // An unterminated quote falls back to the unquoted handling on the @@ -72,7 +72,7 @@ func TestParseServicesFilters(t *testing.T) { } } -// 1077 means "no attempt to start since boot" — a clean stopped service, not a +// 1077 means "no attempt to start since boot" - a clean stopped service, not a // failure, and reporting it red would cry wolf on every host. func TestParseServicesExitCode1077(t *testing.T) { in := `[{"Name":"Idle","DisplayName":"Idle","State":"Stopped","StartMode":"Auto","PathName":"C:\\Idle\\i.exe","ExitCode":1077}]` diff --git a/internal/workloads/workloads.go b/internal/workloads/workloads.go index c516d3f..c0a55ac 100644 --- a/internal/workloads/workloads.go +++ b/internal/workloads/workloads.go @@ -37,7 +37,7 @@ func Collect(ctx context.Context) Result { // // It sorts first: `docker ps` output ordering is not stable, and an // ordering-sensitive hash would resend the full list every 60 seconds forever -// — a cost visible only as traffic. +// - a cost visible only as traffic. // // StartedAt is deliberately excluded: it does not change while a container // runs, and including it would add nothing. Restarts IS included, because a