From fab9d11c7e078519e6936704ef02b223d14ea7b8 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Thu, 17 Sep 2026 07:48:36 +0000 Subject: [PATCH] docs(plan): fix task cross-references and error sentinel placement --- .../2026-09-17-metric-alerts-heartbeats.md | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md b/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md index f8ff462..875c469 100644 --- a/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md +++ b/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md @@ -150,7 +150,7 @@ type MonitorServerState struct { } ``` -Add `RebootRequiredSince *time.Time \`bson:"reboot_required_since,omitempty" json:"reboot_required_since,omitempty"\`` to `Inventory` in `server/internal/models/server.go` (used in Task 6; adding it now keeps model changes in one commit). +Add `RebootRequiredSince *time.Time \`bson:"reboot_required_since,omitempty" json:"reboot_required_since,omitempty"\`` to `Inventory` in `server/internal/models/server.go` (used in Task 5; adding it now keeps model changes in one commit). - [ ] **Step 2: Write the failing tests** @@ -399,7 +399,8 @@ git commit -m "refactor(monitors): extract applyTransition and add passive monit - `func RotateHeartbeatToken(instanceID, monitorID string) (string, error)` - `func heartbeatVerdict(m models.Monitor, now time.Time) (down bool, message string)` - `func SweepHeartbeats(now time.Time)` - - `func validateHeartbeat(t *models.MonitorTarget) error` + - `func validateHeartbeat(t *models.MonitorTarget) error` (errors wrap `ErrInvalidMonitor`) + - `var ErrInvalidMonitor = errors.New("invalid monitor")` - `CreateMonitor` now returns the plaintext token via a new field on its result: add `HeartbeatToken string \`bson:"-" json:"heartbeat_token,omitempty"\`` to `models.Monitor`. - [ ] **Step 1: Write the failing tests** @@ -535,6 +536,9 @@ const defaultHeartbeatGraceSec = 300 var ErrHeartbeatNotFound = errors.New("heartbeat not found") +// ErrInvalidMonitor marks a validation failure, which handlers answer with 400. +var ErrInvalidMonitor = errors.New("invalid monitor") + func NewHeartbeatToken() (string, string, error) { raw := make([]byte, 24) if _, err := rand.Read(raw); err != nil { @@ -551,10 +555,10 @@ func HashHeartbeatToken(token string) string { func validateHeartbeat(t *models.MonitorTarget) error { if t.PeriodSec < 60 { - return fmt.Errorf("period_sec must be at least 60") + return fmt.Errorf("%w: period_sec must be at least 60", ErrInvalidMonitor) } if t.GraceSec < 0 { - return fmt.Errorf("grace_sec must not be negative") + return fmt.Errorf("%w: grace_sec must not be negative", ErrInvalidMonitor) } if t.GraceSec == 0 { t.GraceSec = defaultHeartbeatGraceSec @@ -759,7 +763,7 @@ In `CreateMonitor`, after the group normalisation: } ``` -(Metric validation is added in Task 8.) Add to `models.Monitor`: +(Metric validation is added in Task 7.) Add to `models.Monitor`: ```go // HeartbeatToken is the plaintext token, set only on the create response. @@ -822,7 +826,7 @@ git commit -m "feat(monitors): heartbeat token, ping recording and overdue verdi **Interfaces:** - Consumes: `services.RecordHeartbeat`, `services.RotateHeartbeatToken`, `services.SweepHeartbeats`, `services.ErrHeartbeatNotFound`, `services.MaxHeartbeatBody`. -- Produces: routes `GET|POST /public/hb/:token`, `GET|POST /public/hb/:token/:kind`, `POST /api/monitors/:id/rotate-token` returning `{"heartbeat_token": string}`; `metricsched.Start(ctx)`; exported `metricsched.Sweep func(now time.Time)` hook list used in Task 9. +- Produces: routes `GET|POST /public/hb/:token`, `GET|POST /public/hb/:token/:kind`, `POST /api/monitors/:id/rotate-token` returning `{"heartbeat_token": string}`; `metricsched.Start(ctx)`. - [ ] **Step 1: Write the failing handler test** @@ -1008,7 +1012,7 @@ func rotateHeartbeatToken(c *gin.Context) { Also check that `RequireActiveLicense` in `licence.go` gates the rotate POST (deny by default). Leave it gated, matching create/update. -In `createMonitor` in `api/monitors.go`, map validation errors to 400 instead of 500. Services return `fmt.Errorf` for validation, so wrap them: add `var ErrInvalidMonitor = errors.New("invalid monitor")` in services, return `fmt.Errorf("%w: period_sec must be at least 60", ErrInvalidMonitor)` from `validateHeartbeat` (and later from the metric validators), and in the handler: +In `createMonitor` in `api/monitors.go`, map validation errors to 400 instead of 500. `validateHeartbeat` wraps `services.ErrInvalidMonitor` (Task 2), and so do the metric validators later. In the handler: ```go if errors.Is(err, services.ErrInvalidMonitor) { @@ -1017,7 +1021,7 @@ In `createMonitor` in `api/monitors.go`, map validation errors to 400 instead of } ``` -Apply the same mapping in `updateMonitor`. Update `TestValidateHeartbeat` to also assert `errors.Is(err, ErrInvalidMonitor)`. +Apply the same mapping in `updateMonitor`. - [ ] **Step 5: Create `metricsched`** @@ -2069,7 +2073,7 @@ git commit -m "feat(monitors): sweep metric monitors per server with incidents p **Interfaces:** - Consumes: `ListMonitorServerStates`, `VisibleServerIDs`, `ListServers` or `ListServersFiltered` for hostnames. -- Produces: `GET /api/monitors/:id/servers` returning `[]models.MonitorServerState` with `hostname`; `func filterByVisibleServer[T any](items []T, serverID func(T) string, visible map[string]bool, restricted bool) []T`. +- Produces: `GET /api/monitors/:id/servers` returning `[]models.MonitorServerState` with `hostname`; `func FilterByVisibleServer[T any](items []T, serverID func(T) string, visible map[string]bool, restricted bool) []T`. - [ ] **Step 1: Failing test** @@ -2086,10 +2090,10 @@ func TestFilterByVisibleServer(t *testing.T) { incs := []models.Incident{{IncidentID: "a"}, {IncidentID: "b", ServerID: "s1"}, {IncidentID: "c", ServerID: "s2"}} id := func(i models.Incident) string { return i.ServerID } - if got := filterByVisibleServer(incs, id, nil, false); len(got) != 3 { + if got := FilterByVisibleServer(incs, id, nil, false); len(got) != 3 { t.Fatalf("unrestricted keeps all, got %d", len(got)) } - got := filterByVisibleServer(incs, id, map[string]bool{"s1": true}, true) + got := FilterByVisibleServer(incs, id, map[string]bool{"s1": true}, true) if len(got) != 2 || got[0].IncidentID != "a" || got[1].IncidentID != "b" { t.Fatalf("restricted keeps serverless and visible only, got %+v", got) } @@ -2099,7 +2103,7 @@ func TestFilterByVisibleServer(t *testing.T) { - [ ] **Step 2: Run to verify failure** Run: `cd server && go test ./internal/services/ -run TestFilterByVisibleServer` -Expected: `undefined: filterByVisibleServer`. +Expected: `undefined: FilterByVisibleServer`. - [ ] **Step 3: Implement the filter** @@ -2122,7 +2126,6 @@ func FilterByVisibleServer[T any](items []T, serverID func(T) string, visible ma } ``` -Export it as `FilterByVisibleServer` and call it that in the test too (the test is in package `services`, so rename the call in Step 1's test). - [ ] **Step 4: Handlers**