stability: protect pkg/flag globals from concurrent mutation #4

Open
opened 2026-05-23 18:30:16 +00:00 by Klaus · 0 comments
Owner

Problem

pkg/flag/flag.go exposes mutable package-level variables (Token, Host, Port, Mode, Debug, Insecure, etc.) that are set once at startup but later read concurrently by every tool handler. Tests (e.g. operation/pull/pull_test.go) directly mutate flag.Token, creating data races against any in-flight tool call when go test -race is run alongside the HTTP server.

Suggested direction

Two options:

  1. Lightweight: wrap each global with sync/atomic or a sync.RWMutex and expose Get/Set accessors. Lowest churn.
  2. Cleaner: move config into an immutable Config struct constructed once in cmd/cmd.go and passed via context (matches how the per-request token already flows through pkg/context.TokenContextKey).

Either way: run the test suite with -race in CI to catch regressions.

## Problem `pkg/flag/flag.go` exposes mutable package-level variables (`Token`, `Host`, `Port`, `Mode`, `Debug`, `Insecure`, etc.) that are set once at startup but later read concurrently by every tool handler. Tests (e.g. `operation/pull/pull_test.go`) directly mutate `flag.Token`, creating data races against any in-flight tool call when `go test -race` is run alongside the HTTP server. ## Suggested direction Two options: 1. **Lightweight:** wrap each global with sync/atomic or a `sync.RWMutex` and expose `Get`/`Set` accessors. Lowest churn. 2. **Cleaner:** move config into an immutable `Config` struct constructed once in `cmd/cmd.go` and passed via context (matches how the per-request token already flows through `pkg/context.TokenContextKey`). Either way: run the test suite with `-race` in CI to catch regressions.
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Klaus/gitea-mcp#4