Add a start/stop controller for the S2 storage sink

[archandatta]

Summary

• add S2StorageController in server/lib/events/s2storage.go, a start/stop wrapper around S2StorageWriter that resolves the stream name through streamFn at Start
• keep empty S2 config as a no-op without resolving the stream, and open at most one writer per process after a writer has successfully started
• expose Running() and EverStarted() so later callers can distinguish active forwarding from any prior S2 persistence
• cover never-start, repeated and concurrent Start, failed-start rollback, empty config, Stop without Start, and post-Stop state with unit tests

Why

An upcoming telemetry mode needs a browser instance to forward selected events without ever opening its S2 append session. cmd/api/main.go still opens S2StorageWriter directly at the existing boot and fork-identity start points; this branch only adds the lifecycle owner that a later change can wire in.

Existing behavior is unchanged: main.go, api.go, S2StorageWriter, StorageWriter, and S2 read-from-seq-0 behavior are untouched.

Testing

• cd server && GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache go test ./lib/events/ -run '^TestS2StorageController_' -count=20 -race -shuffle=on — passed, including concurrent failed-start result propagation
• cd server && GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache go test ./lib/events/ -count=1 -race — ok, includes new controller lifecycle and concurrency tests
• cd server && GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache go build ./... — passed
• cd server && GOCACHE=/tmp/go-build-cache GOMODCACHE=/tmp/go-mod-cache go vet ./... — no findings
• DOCKER_BUILDKIT=1 docker build -f images/chromium-headless/image/Dockerfile -t kernel-headless-test . — succeeded, image sha256:810e301828db810062764099944738bdb2e647978f0c66d68fb3f7d72141b078
• headless container with S2_BASIN, S2_ACCESS_TOKEN, and S2_STREAM unset — GET /spec.yaml 200, zero S2 storage lines in /var/log/supervisord/kernel-images-api
• headless container with fake S2_BASIN, S2_ACCESS_TOKEN, and S2_STREAM — GET /spec.yaml 200, one S2 storage enabled line in /var/log/supervisord/kernel-images-api
• telemetry API by curl in both S2 modes — PUT /telemetry 201, GET /telemetry 200, POST /telemetry/events 200, and GET /telemetry/stream?replay=all delivered the posted event frame
• docker stop -t 30 in both S2 modes — clean shutdown signal received, zero drain incomplete / drain deadline exceeded warnings

Not run: real S2 credentials; none were available, so the S2-enabled live check used fake values and produced the expected submit ack error after posting an event.

---

Note

Low Risk
Additive lifecycle wrapper and tests only; no changes to existing boot paths or S2 writer behavior until a follow-up wires the controller in.

Overview
Introduces S2StorageController, a mutex-guarded start/stop owner around S2StorageWriter so S2 append sessions can be opened only when needed (e.g. telemetry mode) instead of at boot.

Start resolves the stream via streamFn (for forks that learn the stream later), skips work when basin/token or stream are empty, opens at most one writer, and serializes concurrent starts. Stop drains the writer, honors context during in-flight start/stop, and leaves EverStarted() true after a successful start so callers can tell if anything may have been persisted—even though a later Start after Stop does not reopen the sink.

Adds unit tests for idempotency, concurrency, failed-start rollback, logging, and stop/context behavior. Production wiring is unchanged in this PR; main still uses S2StorageWriter directly.

^{Reviewed by Cursor Bugbot for commit 7081930. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[Sayan-]

• p1: S2StorageController.Stop clears its writer even when shutdown exits before draining or closing storage. everStarted then prevents reopening, and later stops cannot reach the live writer. Reproduced 20/20 under race with a blocked append.
• p2: the controller holds mu across streamFn, writer startup, and the full stop. A blocked callback or stop makes Stop(ctx) exceed its deadline while waiting for the lock and blocks both state accessors. Reproduced 20/20 under race.
• p2: Start logs “S2 storage enabled” before startup succeeds. A canceled parent returns an error with both state flags false while retaining the enabled log. Reproduced 20/20 with captured logging.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 4e921bb. Configure here.}

[cursor]

Writer can start after Stop returns

Medium Severity

If Stop times out while Start is still opening the append session, Start still publishes the writer afterward. Running is false when Stop returns, so a caller can treat the sink as down and later see events forwarded anyway.

Additional Locations (1)

• server/lib/events/s2storage.go#L296-L298

 

^{Reviewed by Cursor Bugbot for commit 4e921bb. Configure here.}

[Sayan-]

• p2: concurrent Start calls report success before the in-flight start finishes. If the leader later fails, the follower has already returned nil while Running() and EverStarted() remain false. This reproduces under -race and is not covered by the current successful-concurrency test.

[Sayan-]

yeet