From 5b238b3f549693626ec9eb734c2058233cd8e97c Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Tue, 4 Aug 2026 20:24:05 +0200 Subject: [PATCH] docs: align historical plans/specs with renamed identifiers (roundTrip -> execute, log schema) Co-Authored-By: Claude Fable 5 --- .../plans/2026-07-25-garmin-direct-wrapper-plan.md | 8 ++++---- .../plans/2026-07-26-structured-json-logging.md | 14 +++++++------- .../plans/2026-07-27-workout-not-found-plan.md | 8 ++++---- .../2026-07-26-structured-json-logging-design.md | 6 +++--- .../specs/2026-07-27-workout-not-found-design.md | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/superpowers/plans/2026-07-25-garmin-direct-wrapper-plan.md b/docs/superpowers/plans/2026-07-25-garmin-direct-wrapper-plan.md index ac1422c..c93b022 100644 --- a/docs/superpowers/plans/2026-07-25-garmin-direct-wrapper-plan.md +++ b/docs/superpowers/plans/2026-07-25-garmin-direct-wrapper-plan.md @@ -415,7 +415,7 @@ EOF **Interfaces:** - Consumes: nothing from other tasks. -- Produces: `Config{PythonPath, GarminEmail, GarminPassword, TokenStorePath string}`, `NewClient(cfg Config) Client`, `subprocessClient` (unexported struct implementing `Client`), `wireRequest{ID int; Cmd string; Params any}`, `wireResponse{ID int; Result json.RawMessage; Error string}`, `callParams{Method string; Args map[string]any}`, `(*subprocessClient).roundTrip(cmd string, params any) (json.RawMessage, error)`, `(*subprocessClient).ensureStarted() error`, `(*subprocessClient).close() error` (unexported, no-lock; `Close()`/`UpdateCredentials` call it while already holding `c.mu`). Later tasks (3, 4) add methods to `subprocessClient` using `roundTrip`/`ensureStarted` and must not redefine these types. +- Produces: `Config{PythonPath, GarminEmail, GarminPassword, TokenStorePath string}`, `NewClient(cfg Config) Client`, `subprocessClient` (unexported struct implementing `Client`), `wireRequest{ID int; Cmd string; Params any}`, `wireResponse{ID int; Result json.RawMessage; Error string}`, `callParams{Method string; Args map[string]any}`, `(*subprocessClient).roundTrip(cmd string, params any) (json.RawMessage, error)`, `(*subprocessClient).ensureStarted() error`, `(*subprocessClient).close() error` (unexported, no-lock; `Close()`/`UpdateCredentials` call it while already holding `c.mu`). Later tasks (3, 4) add methods to `subprocessClient` using `execute`/`ensureStarted` and must not redefine these types. - [ ] **Step 1: Write the failing tests for the transport layer** @@ -576,7 +576,7 @@ func TestSubprocessClient_Close_NoOpWhenNotStarted(t *testing.T) { - [ ] **Step 2: Run the tests to verify they fail** Run: `cd backend && go test ./internal/garmin/... -run TestSubprocessClient -v` -Expected: compile failure — `subprocessClient`, `wireResponse`, `roundTrip`, `maxWrapperLineBytes`, `Config` (new shape) don't exist yet. +Expected: compile failure — `subprocessClient`, `wireResponse`, `execute`, `maxWrapperLineBytes`, `Config` (new shape) don't exist yet. - [ ] **Step 3: Rewrite `client.go`** @@ -879,7 +879,7 @@ EOF - Modify: `backend/internal/garmin/client_test.go` **Interfaces:** -- Consumes: `subprocessClient`, `roundTrip`, `ensureStarted`, `mapAuthStatus`, `authResultWire`, `newFakeWrapperClient`/`fakeResult`/`fakeError` from Task 2. +- Consumes: `subprocessClient`, `execute`, `ensureStarted`, `mapAuthStatus`, `authResultWire`, `newFakeWrapperClient`/`fakeResult`/`fakeError` from Task 2. - Produces: `(*subprocessClient).Authenticate(ctx) (AuthResult, error)`, `(*subprocessClient).CompleteMFA(ctx, code) (AuthResult, error)`. - [ ] **Step 1: Write the failing tests** @@ -992,7 +992,7 @@ Expected: compile failure — `Authenticate`/`CompleteMFA` methods don't exist o - [ ] **Step 3: Implement `Authenticate`/`CompleteMFA`** -Append to `backend/internal/garmin/client.go` (after `roundTrip`): +Append to `backend/internal/garmin/client.go` (after `execute`): ```go func (c *subprocessClient) Authenticate(ctx context.Context) (AuthResult, error) { diff --git a/docs/superpowers/plans/2026-07-26-structured-json-logging.md b/docs/superpowers/plans/2026-07-26-structured-json-logging.md index 470e910..2b0a834 100644 --- a/docs/superpowers/plans/2026-07-26-structured-json-logging.md +++ b/docs/superpowers/plans/2026-07-26-structured-json-logging.md @@ -4,14 +4,14 @@ **Goal:** Emit structured JSON logs on stdout for (1) every HTTP API request and (2) every Garmin wrapper subprocess call, correlated by a per-request id — without touching any existing `log.Printf` call site. -**Architecture:** A new `internal/applog` package wraps `log/slog` with a JSON handler plus context helpers (`WithLogger`/`FromContext`). `internal/api` gains a `loggingMiddleware` that logs one line per HTTP request and stashes a request-id-tagged logger into the request context. `internal/garmin`'s `client.go` reads that same logger back out of context (via the `ctx` every `Client` method already receives) inside `roundTrip`, the single funnel point all six Garmin calls go through, logging one line per subprocess round-trip. +**Architecture:** A new `internal/applog` package wraps `log/slog` with a JSON handler plus context helpers (`WithLogger`/`FromContext`). `internal/api` gains a `loggingMiddleware` that logs one line per HTTP request and stashes a request-id-tagged logger into the request context. `internal/garmin`'s `client.go` reads that same logger back out of context (via the `ctx` every `Client` method already receives) inside `execute`, the single funnel point all six Garmin calls go through, logging one line per subprocess round-trip. **Tech Stack:** Go stdlib `log/slog` (no new dependency), `github.com/go-chi/chi/v5/middleware` (already-available subpackage of the existing chi dependency). ## Global Constraints - Existing `log.Printf`/`log.Fatalf` call sites are untouched — they keep going to stderr, unstructured, exactly as today. -- Garmin credentials must never be logged. Confirmed safe: `authenticate`/`complete_mfa`/`call` wire params never carry email/password (only env vars at subprocess spawn do) — logging `params` in full is safe everywhere in `roundTrip`. +- Garmin credentials must never be logged. Confirmed safe: `authenticate`/`complete_mfa`/`call` wire params never carry email/password (only env vars at subprocess spawn do) — logging `params` in full is safe everywhere in `execute`. - `gofmt -l .` must report nothing; `go vet ./...` and `go build ./...` must pass. --- @@ -480,14 +480,14 @@ git commit -m "feat(api): add structured JSON access log with request-id correla ### Task 3: Garmin wrapper call log **Files:** -- Modify: `backend/internal/garmin/client.go` (`roundTrip`, `ensureStarted`, all six call sites) -- Modify: `backend/internal/garmin/client_test.go` (update 3 direct-`roundTrip` calls, add 2 new tests) +- Modify: `backend/internal/garmin/client.go` (`execute`, `ensureStarted`, all six call sites) +- Modify: `backend/internal/garmin/client_test.go` (update 3 direct-`execute` calls, add 2 new tests) **Interfaces:** - Consumes: `applog.FromContext` (Task 1). - Produces: `func (c *subprocessClient) roundTrip(ctx context.Context, cmdName string, params any) (result json.RawMessage, err error)` (signature change — `ctx` added as first param), `func (c *subprocessClient) ensureStarted(ctx context.Context) error` (signature change — `ctx` added). -- [ ] **Step 1: Update the 3 existing direct-`roundTrip` tests and write the 2 new failing tests** +- [ ] **Step 1: Update the 3 existing direct-`execute` tests and write the 2 new failing tests** In `backend/internal/garmin/client_test.go`, add `"bytes"` and `"log/slog"` to the stdlib import block, and add `"geniusrun/backend/internal/applog"` as a new import group. @@ -591,7 +591,7 @@ func TestSubprocessClient_RoundTrip_LogsErrorAtWarnLevel(t *testing.T) { - [ ] **Step 2: Run tests to verify they fail** Run: `cd backend && go test ./internal/garmin/... -run 'TestSubprocessClient_RoundTrip' -v` -Expected: FAIL — `roundTrip` still takes 2 args, not 3; `applog` import unresolved. +Expected: FAIL — `execute` still takes 2 args, not 3; `applog` import unresolved. - [ ] **Step 3: Update `client.go`** @@ -690,7 +690,7 @@ to: } ``` -Replace `roundTrip` entirely. From: +Replace `execute` entirely. From: ```go // roundTrip sends one request and returns its result payload, or an error diff --git a/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md b/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md index 1b82d62..a18d468 100644 --- a/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md +++ b/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md @@ -165,14 +165,14 @@ EOF ### Task 2: `internal/garmin/client.go` exposes `ErrNotFound` **Files:** -- Modify: `backend/internal/garmin/client.go` (`wireResponse`, new `ErrNotFound`, `roundTrip`) +- Modify: `backend/internal/garmin/client.go` (`wireResponse`, new `ErrNotFound`, `execute`) - Modify: `backend/internal/garmin/client_test.go` (`wireResponsePayload`/harness, new test) **Interfaces:** - Consumes: `wireResponse.NotFound` (wire field written by Task 1's `wrapper.py` change). - Produces: `var ErrNotFound error` -- Task 4 (`fillPendingWorkouts`) checks `errors.Is(err, garmin.ErrNotFound)` against errors returned by `GetWorkoutByID` (and, since - this is wired at the generic `roundTrip` level, any other `Client` method too). + this is wired at the generic `execute` level, any other `Client` method too). - [ ] **Step 1: Write the failing test** @@ -256,7 +256,7 @@ already added it to the test file; the actual compile failure is `wireResponse` `NotFound` yet, and `ErrNotFound` is undefined) -- confirms the test exercises code that doesn't exist yet. -- [ ] **Step 3: Add `ErrNotFound` and wire it through `roundTrip`** +- [ ] **Step 3: Add `ErrNotFound` and wire it through `execute`** In `backend/internal/garmin/client.go`, replace: @@ -289,7 +289,7 @@ type wireResponse struct { var ErrNotFound = errors.New("garmin: resource not found") ``` -Replace, in `roundTrip`: +Replace, in `execute`: ```go if resp.Error != "" { diff --git a/docs/superpowers/specs/2026-07-26-structured-json-logging-design.md b/docs/superpowers/specs/2026-07-26-structured-json-logging-design.md index 0801200..140a3b3 100644 --- a/docs/superpowers/specs/2026-07-26-structured-json-logging-design.md +++ b/docs/superpowers/specs/2026-07-26-structured-json-logging-design.md @@ -100,7 +100,7 @@ preflights: ### Garmin wrapper call log (`internal/garmin`) -`client.go`'s `roundTrip` is the single funnel point every one of the six +`client.go`'s `execute` is the single funnel point every one of the six `Client` methods already goes through (confirmed while surveying: none of them ever put Garmin credentials in the wire `params` -- email/password only ever reach the subprocess via env vars at spawn time, so logging @@ -134,7 +134,7 @@ what keeps this safe for the huge per-second-telemetry responses while still showing tiny auth results (`{"status":"mfa_required",...}`) in full -- exactly the detail needed for the motivating incident. -All six `Client` methods pass their own `ctx` through to `roundTrip` +All six `Client` methods pass their own `ctx` through to `execute` (currently they call it without one). `ensureStarted` also gains a `ctx context.Context` param and logs one `Info` line on actual spawn (`python_path`, whether a token store is configured) -- spawning the @@ -163,7 +163,7 @@ subprocess is itself a call to the external component. `applog.WithLogger`, call `Authenticate`/`CompleteMFA`, assert the emitted JSON line's `cmd`/`duration_ms`/`result_preview` fields, and that a wrapper-reported error surfaces at `Warn` with an `error` field. Update - the three existing tests that call `roundTrip` directly + the three existing tests that call `execute` directly (`TestSubprocessClient_RoundTrip_DetectsIDMismatch`, `_SubprocessClosedIsError`, `_WrapperErrorPropagates`) to pass `context.Background()` as the new first argument. diff --git a/docs/superpowers/specs/2026-07-27-workout-not-found-design.md b/docs/superpowers/specs/2026-07-27-workout-not-found-design.md index e74ad8f..816de79 100644 --- a/docs/superpowers/specs/2026-07-27-workout-not-found-design.md +++ b/docs/superpowers/specs/2026-07-27-workout-not-found-design.md @@ -42,7 +42,7 @@ dispatcher that `get_workout_by_id` goes through) gains a check: if the caught e - `wireResponse` gains `NotFound bool `json:"not_found,omitempty"``. - A new sentinel: `var ErrNotFound = errors.New("garmin: resource not found")`. -- `roundTrip`: when `resp.Error != "" && resp.NotFound`, the returned error wraps `ErrNotFound` +- `execute`: when `resp.Error != "" && resp.NotFound`, the returned error wraps `ErrNotFound` (`fmt.Errorf("%s: %s: %w", cmdName, resp.Error, ErrNotFound)`), so callers can `errors.Is(err, garmin.ErrNotFound)` regardless of which method was called. @@ -96,7 +96,7 @@ no shape change -- a test simulating a 404 just sets the mapped error to - `internal/garmin/pyscript/tests/test_wrapper.py`: a new test mirroring the existing `test_call_propagates_garminconnect_exception_as_error`, but raising `GarminConnectNotFoundError` and asserting the response includes `"not_found": true`. -- `internal/garmin` (Go): a test asserting `roundTrip` wraps the error with `ErrNotFound` when +- `internal/garmin` (Go): a test asserting `execute` wraps the error with `ErrNotFound` when the wire response sets `not_found: true`. - `internal/store`: extend the existing `ActivitiesMissingWorkout` test (or add a new one) with a fixture that has `workout_not_found_at` set, asserting it's excluded from both