amadan
gates: a race target, and a deadline on every tick pass
W-6 and W-12. W-6: this is the first slice with real concurrency — a hub behind a mutex, a tick goroutine, one goroutine per stream, three atomics — and the one gate the team runs on every push could not see a data race: `-race` requires cgo, and AGENTS.md keeps the repo cgo-free. `make race` sets CGO_ENABLED=1 for a test binary that never ships, which is why it is the one exception, and AGENTS.md now says so and when to run it. `make ci` is unchanged and still cgo-free. The CI step (.amadan/ci.d/50-race) is present but guarded: the cloud runner's provisioning installs the Go toolchain and Node and never names a C compiler, and a step that failed on every push would be worse than no step. It skips loudly and says how to remove the guard. Recorded in the slice-2 record. W-12: `tickForever` handed the housekeeping pass the process context and nothing else, so a pass had no bound of its own while doing one serial standing read per joined participation. A driver that stopped answering would stall §2.4's quiet case for as long as it stayed stuck. Each pass now runs under a deadline of `tickEvery`, with a test that a read which never returns on its own ends with the pass. Verified by mutation: replacing the timeout with a plain cancel fails the test in five seconds.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
5 files changed,
+141
−2
.amadan/ci.d/50-race+19 −0AGENTS.md+6 −0Makefile+14 −1internal/meet/app.go+17 −1internal/meet/tick_test.go+85 −0
diff --git a/.amadan/ci.d/50-race b/.amadan/ci.d/50-race| new file mode 100755 |
| index 0000000..c491592 |
| --- /dev/null |
| +++ b/.amadan/ci.d/50-race |
| @@ -0,0 +1,19 @@ |
| +#!/bin/sh |
| +# The race gate, as its own reported step. It is not part of `make ci`, |
| +# which AGENTS.md keeps cgo-free; `-race` refuses to build without cgo, |
| +# so this is the one step that sets CGO_ENABLED=1. It builds a test |
| +# binary and never a release. |
| +# |
| +# The guard: `-race` needs a C compiler, and the cloud runner's |
| +# provisioning (amadan/docs/ci-cloud-runner.md) installs the Go |
| +# toolchain and Node and never names one. A step that failed on every |
| +# push would be worse than no step, so this skips loudly instead. |
| +# When somebody confirms cc on the runner, delete the guard — a race |
| +# gate that can silently skip is half a gate. |
| +set -e |
| +if ! command -v cc >/dev/null 2>&1 && ! command -v gcc >/dev/null 2>&1; then |
| + echo "race: SKIPPED — no C compiler on this runner, and -race requires cgo." |
| + echo "race: install one (apt-get install -y gcc) and delete the guard in .amadan/ci.d/50-race." |
| + exit 0 |
| +fi |
| +exec make race |
diff --git a/AGENTS.md b/AGENTS.md| index 88071db..ed363e8 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -55,6 +55,12 @@ mechanically. |
| - The gate is `make ci` (vet + gofmt + tests) — the same definition |
| CI runs. Run it before every push. `CGO_ENABLED=0` throughout: the |
| stack is cgo-free by design. |
| +- `make race` is the one exception to that rule, and the third gate: |
| + `-race` refuses to build without cgo, so it sets `CGO_ENABLED=1`. It |
| + builds a test binary and never a release, so nothing that ships gains |
| + a C dependency. Run it for any change to `internal/meet/hub` or the |
| + stream — a hub behind a mutex, a tick goroutine and one goroutine per |
| + stream is more concurrency than `make ci` can see. |
| - `make browser` is the second gate, for any change to a page or to the |
| client: a real call in real Chromium. It needs the Claude Code bash |
| sandbox **off** — the sandbox blocks UDP, so Chromium gathers no ICE |
diff --git a/Makefile b/Makefile| index 4cd3ddd..6927e6d 100644 |
| --- a/Makefile |
| +++ b/Makefile |
| @@ -16,7 +16,7 @@ RELEASE_BIN := releases/$(APP)-$(RELEASE_GOOS)-$(RELEASE_GOARCH) |
| # first commit, name it yourself: make release VERSION=v0.1.0 |
| VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null) |
| -.PHONY: build release version-check test vet fmt-check migration-check ci browser |
| +.PHONY: build release version-check test vet fmt-check migration-check ci race browser |
| # build is the compile check: with more than one package matched, go |
| # build discards the output, so this catches a broken package without |
| @@ -63,6 +63,19 @@ test: |
| vet: |
| go vet ./... |
| +# The race gate, and the one place in this repo that sets CGO_ENABLED=1. |
| +# `-race` refuses to build without cgo, and AGENTS.md's cgo-free rule is |
| +# about what ships: this builds a test binary and never a release, so the |
| +# exception costs the deployed artifact nothing. It is not folded into |
| +# `ci` for that reason — `ci` must stay cgo-free — and runs as its own |
| +# CI step (.amadan/ci.d/50-race) where the runner has a C compiler. |
| +# |
| +# Slice 2 is the first slice with real concurrency: a hub behind a mutex, |
| +# a tick goroutine, one goroutine per stream and three atomics. Run this |
| +# before every push that touches internal/meet/hub or the stream. |
| +race: |
| + CGO_ENABLED=1 go test -race ./internal/... |
| + |
| fmt-check: |
| @out=$$(gofmt -l .); if [ -n "$$out" ]; then echo "gofmt needed:"; echo "$$out"; exit 1; fi |
diff --git a/internal/meet/app.go b/internal/meet/app.go| index 81fd24b..ffb2daf 100644 |
| --- a/internal/meet/app.go |
| +++ b/internal/meet/app.go |
| @@ -239,11 +239,27 @@ func (a *app) tickForever(ctx context.Context) { |
| case <-ctx.Done(): |
| return |
| case <-t.C: |
| - a.hub.Tick(ctx) |
| + a.tickOnce(ctx, tickEvery) |
| } |
| } |
| } |
| +// tickOnce is one housekeeping pass under a deadline of its own (review |
| +// W-12). The read phase does one standing lookup per joined |
| +// participation, serially, with the hub lock down; given only the |
| +// process context, a driver that stops answering would stall §2.4's |
| +// quiet case for as long as it stayed stuck, and a removed member would |
| +// then be dropped only by the next thing they did. A pass that runs |
| +// past the interval has already lost its slot, so `within` is that |
| +// interval: the reads that did answer are applied, the ones that did |
| +// not wait for the next pass, which is what Tick already does with a |
| +// failed lookup. |
| +func (a *app) tickOnce(ctx context.Context, within time.Duration) { |
| + ctx, cancel := context.WithTimeout(ctx, within) |
| + defer cancel() |
| + a.hub.Tick(ctx) |
| +} |
| + |
| // build wires the whole app: refusals, schema, router, static files. It |
| // returns the *app alongside the mux so tests can reach into it (see |
| // NewForTest); production code uses App, which discards the *app. |
diff --git a/internal/meet/tick_test.go b/internal/meet/tick_test.go| new file mode 100644 |
| index 0000000..77da4ba |
| --- /dev/null |
| +++ b/internal/meet/tick_test.go |
| @@ -0,0 +1,85 @@ |
| +package meet |
| + |
| +import ( |
| + "context" |
| + "io" |
| + "log/slog" |
| + "sync/atomic" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/oficina/meet/internal/meet/hub" |
| +) |
| + |
| +// Review W-12: the fifteen-second housekeeping pass reads standing once |
| +// per joined participation, serially, and it used to be given the |
| +// process context and nothing else. A driver that stopped answering |
| +// would then stall §2.4's quiet case for as long as it stayed stuck — |
| +// no lapsed resume windows collected, no kind changes applied, and a |
| +// removed member dropped only by the next thing they did. |
| +// |
| +// The property under test is that the pass ends on its own deadline. |
| +// `within` is a parameter so this costs milliseconds rather than the |
| +// fifteen seconds tickForever passes. |
| +func TestATickPassEndsOnItsOwnDeadline(t *testing.T) { |
| + var stuck atomic.Bool |
| + reached := make(chan struct{}, 1) |
| + h := hub.New(hub.Config{ |
| + Now: time.Now, |
| + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), |
| + StandingFor: func(ctx context.Context, room string, personID int64) (hub.Kind, bool, error) { |
| + if !stuck.Load() { |
| + return hub.KindMember, true, nil |
| + } |
| + select { |
| + case reached <- struct{}{}: |
| + default: |
| + } |
| + // A read that never comes back on its own: the only thing |
| + // that ends it is the context the tick handed down. |
| + <-ctx.Done() |
| + return 0, false, ctx.Err() |
| + }, |
| + }) |
| + p, err := h.Open("r1", "s1", 1, "Mia", hub.KindMember) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // Joined, so the tick has a standing to read for it. |
| + if err := h.Join(context.Background(), "r1", "s1", p.Peer); err != nil { |
| + t.Fatal(err) |
| + } |
| + stuck.Store(true) |
| + |
| + a := &app{hub: h} |
| + done := make(chan struct{}) |
| + started := time.Now() |
| + go func() { |
| + a.tickOnce(context.Background(), 200*time.Millisecond) |
| + close(done) |
| + }() |
| + |
| + select { |
| + case <-done: |
| + case <-time.After(5 * time.Second): |
| + t.Fatal("a tick pass whose standing read never returned did not end on its own deadline") |
| + } |
| + select { |
| + case <-reached: |
| + default: |
| + t.Fatal("the pass never reached a standing read, so it proves nothing about the deadline") |
| + } |
| + if took := time.Since(started); took < 150*time.Millisecond { |
| + t.Fatalf("the pass ended after %v, before its own deadline", took) |
| + } |
| +} |
| + |
| +// The deadline is not decoration: tickForever passes the interval, so a |
| +// pass can never overrun the slot the next one needs. Asserted here |
| +// because the constant and the call are in different places and a |
| +// future edit could quietly separate them. |
| +func TestTheTickDeadlineIsTheTickInterval(t *testing.T) { |
| + if tickEvery != 15*time.Second { |
| + t.Fatalf("tickEvery is %v, and spec §2.4 says fifteen seconds", tickEvery) |
| + } |
| +} |