[frahlg]

internal/ocpp is the package that fails CI most often on unrelated PRs. This PR fixes four causes. One is in Core; three are in the tests.

What failed

Run 40178374052 (PR #1827), attempt 1:

--- FAIL: TestPerChargerCredentialOverTheWire (3.01s)
    server_test.go:75: test charger did not observe the server closing

The same run, attempt 2: the package hit the 300 s timeout. The dump shows the test goroutine parked for 4 minutes in ocppj.(*Client).Stop at client.go:169 (<-cleanupC), called from the cleanup in connectCapabilityCP.

Runs 39786224865 and 39135453604, race job:

WARNING: DATA RACE
Read  ... ws.(*Client).writePump.func1()        websocket.go:913
Previous write ... ws.(*Client).SetDisconnectedHandler()  websocket.go:860
                   ocppj.(*Client).Stop()                 client.go:158

Run 39736794360, race job, TestFailedAndDuplicateHandshakesPreserveCurrentSocket:

WARNING: DATA RACE
Write ... ws.(*Client).cleanup()   (close of the out queue)
Previous read ... ws.(*Client).Write() <- ocppj.(*Client).SendError <- notSupportedError <- readPump
panic: send on closed channel

Same message as the first: runs 39371191559 and 39557352450.

Causes

1. Core: a socket that is still opening survives Server.Stop. ocpp-go closes the sockets in its table when the server stops (stopConnections, run from http.Server.Shutdown). A handshake that has passed our connection check joins that table only after the library writes its 101. If Stop sweeps the table in that gap, the socket stays open on the stopped server. The CI log shows it: OCPP charger connected as pending ... garage-impostor after GetConfiguration send failed ... ocppj server is not started. Core starts and stops the server only once per process, so a running box is not affected; the fix is still in Core, because Stop should close every socket. Server.Stop now refuses new handshakes and waits, at most 2 s, until each one already past the check has joined the table or ended. Patch changeset.
2. Test: ocpp-go v0.19.0's client races with itself on close. ocppj.Client.Stop swaps the disconnect handler while the write pump may be reading it after a server-side close (the race above). If the pump has already called the old handler, Stop waits for a call that never comes (the 300 s timeout). ws.Client.Write checks the connection and then queues without its lock, so a reply to Core's probe sent while the charger stops can hit the closed queue (Flaky api and ocpp tests fail CI on unrelated PRs #1712). Test chargers now dial through chargerSocket (in charger_socket_test.go). It serializes Write with Stop, keeps the disconnect handler itself, and passes a disconnect that has already been reported to the handler Stop installs. stopAfterServer and the per-file stop orders are gone: every charger is stopped by its own cleanup, before the server.
3. Test: a port lost between freePort and the library's bind. freePort releases the port, and another socket can take it before ocpp-go listens. The library then exits that listener without an error, and the old helpers waited 2 s and failed with server did not bind on port …. I saw it locally under load. startOCPP starts every test server, sees the early exit of the listener goroutine and tries fresh ports, up to 5 times.
4. Test: identity-probe tests that measured wall-clock windows. TestIdentityProbeTimeoutAndReconnectAreBounded failed 3 times in 60 loaded runs with reconnect bypassed rate limit: the probe is allowed 30 ms after the first, and the test assumed it would reach the reconnect within 20 ms. It now checks only the lower bound (request k goes at least k intervals after the first connect), which load cannot break. Its stop step and TestIdentityProbeRevocationCancelsPendingRequest check the pending timer with a one-hour grace instead of racing a 1 or 20 ms timer.

Evidence

Two throwaway tests aimed at the windows (not committed), race detector, on this laptop:

Case	master	this branch
A charger dials while the server stops: sockets left open on the stopped server	17 of 337	0 of 343
Server closes, charger stops at once, 12 copies × 250: data races	1 in 3,000 (plain client, the CI trace)	0 in 3,000 (chargerSocket)

Whole package on this branch, race detector:

• 6 parallel copies × -count=34, -cpu=1 (204 runs): all pass. The same load on master, three batches of 60: 7 failed (4 on a lost port, 3 on reconnect bypassed rate limit).
• -count=20 -cpu=1,2,4 (60 runs): all pass.

New tests: TestStopWaitsForAHandshakePastTheCheck, TestStopDoesNotWaitForAConnectedCharger, and a check at the end of TestFailedAndDuplicateHandshakesPreserveCurrentSocket that a failed or refused handshake does not hold up a stop.

go vet ./... passes. Full go test ./internal/... ./cmd/ftw/... passes. CI is green on both commits, including the race job.

Review

An independent review (Sonnet) found no bug in Core: every admitted handshake is counted down exactly once, on success, rejection, failed upgrade and duplicate id, and the lock order is consistent. It found one remaining timing hole in TestIdentityProbeTimeoutAndReconnectAreBounded: a stall of one interval before the reconnect let a second probe go on the first connection. The second commit fixes it, because the first request now waits an hour for its answer, so no timer can fire before the reconnect. The test passes with a 200 ms stall forced at that point. The commit also adds a test for chargerSocket's missed-disconnect path and says in its comment that a Write while the server closes can still race.

Not fixed here

• ocpp-go's server StopConnection sends on the socket's close channel after it drops its lock, so a charger that disconnects at that moment can panic the server. Core calls it when it refuses a second socket for a charger id. Library bug, not seen in CI; worth an upstream report.
• A listener that cannot bind leaves Core's OCPP server silently not listening: the library logs nothing and Start returns success. Real, but separate from these flakes.
• cmd/ftw and cmd/sim-ocpp also use the ocpp-go client and could meet the same client races. Neither has failed CI from them; this PR keeps to internal/ocpp.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T10:54:21.151342Z	aae041a	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.