From 562f425b4efa694215607c5f31a1174f47b24c3d Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 15:16:23 -0500 Subject: [PATCH 01/11] fix test in TestClient_GetReportPage --- go/client_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/go/client_test.go b/go/client_test.go index 8b86cf6..d5253c1 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -160,22 +160,22 @@ func TestClient_GetLatestReport(t *testing.T) { } func TestClient_GetReportPage(t *testing.T) { - expectedInitialTS := uint64(1234567891) + expectedInitialTS := uint64(1234567890) expectedReportPage1 := &ReportPage{ Reports: []*ReportResponse{ - {FeedID: feed1, FullReport: hexutil.Bytes(`report1 payload`)}, - {FeedID: feed1, FullReport: hexutil.Bytes(`report2 payload`)}, + {FeedID: feed1, ObservationsTimestamp: 1234567890, FullReport: hexutil.Bytes(`report1 payload`)}, + {FeedID: feed1, ObservationsTimestamp: 1234567891, FullReport: hexutil.Bytes(`report2 payload`)}, }, - NextPageTS: 1234567899, + NextPageTS: 1234567892, } expectedReportPage2 := &ReportPage{ Reports: []*ReportResponse{ - {FeedID: feed1, FullReport: hexutil.Bytes(`report3 payload`)}, - {FeedID: feed1, FullReport: hexutil.Bytes(`report4 payload`)}, + {FeedID: feed1, ObservationsTimestamp: 1234567892, FullReport: hexutil.Bytes(`report3 payload`)}, + {FeedID: feed1, ObservationsTimestamp: 1234567893, FullReport: hexutil.Bytes(`report4 payload`)}, }, - NextPageTS: 1234567999, + NextPageTS: 1234567894, } ms := newMockServer(func(w http.ResponseWriter, r *http.Request) { From a70c6a47c244085520ae57e0ec587c54970f771e Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 15:31:20 -0500 Subject: [PATCH 02/11] add workflow to run go tests --- .github/workflows/go-tests.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/go-tests.yml diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml new file mode 100644 index 0000000..c1b871c --- /dev/null +++ b/.github/workflows/go-tests.yml @@ -0,0 +1,23 @@ +name: Go Tests + +on: + pull_request: + paths: + - "go/**" + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: "1.24" + + - name: Run tests + working-directory: go + run: go test ./... From 3aff6aebfc7ce2ea63b9aa78a18ae0613b5a41a4 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 15:41:06 -0500 Subject: [PATCH 03/11] read scope --- .github/workflows/go-tests.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index c1b871c..a16dc33 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -1,5 +1,9 @@ name: Go Tests +# Security: grants only the minimal scopes required. +permissions: + contents: read + on: pull_request: paths: From 0097a26394c3dffa6c35f5befbfc11f8917ac774 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:00:19 -0500 Subject: [PATCH 04/11] fix potential race when a stream is Closing (via Close()) and when we're still adding connections to the stream --- go/stream.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/stream.go b/go/stream.go index acd1ec5..026a7a8 100644 --- a/go/stream.go +++ b/go/stream.go @@ -137,7 +137,9 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs return } go s.monitorConn(conn) + s.closingMutex.Lock() s.conns = append(s.conns, conn) + s.closingMutex.Unlock() }() continue } From 9007d39b9f26a989daa0688eb0514fd242d5bf5c Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:01:15 -0500 Subject: [PATCH 05/11] enable race check in tests --- .github/workflows/go-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index a16dc33..8fffb6b 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -24,4 +24,4 @@ jobs: - name: Run tests working-directory: go - run: go test ./... + run: go test -race ./... From 2324efaef95f27b721deb660b820f4e4ce7fc8a5 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:03:58 -0500 Subject: [PATCH 06/11] cache-dependency-path --- .github/workflows/go-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 8fffb6b..5762560 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -21,6 +21,7 @@ jobs: uses: actions/setup-go@v5 with: go-version: "1.24" + cache-dependency-path: go/go.sum - name: Run tests working-directory: go From 51213c83ab2f13195b15ebe40422fb6ad24a9737 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:11:37 -0500 Subject: [PATCH 07/11] use latest actions/checkout@v6 --- .github/workflows/go-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 5762560..188c88c 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -15,7 +15,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v6 - name: Setup Go uses: actions/setup-go@v5 From bf2419e0d11a49460c52dafb14ef5149c4067e62 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:21:35 -0500 Subject: [PATCH 08/11] add comment --- go/stream.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/stream.go b/go/stream.go index 026a7a8..6155b82 100644 --- a/go/stream.go +++ b/go/stream.go @@ -137,6 +137,8 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs return } go s.monitorConn(conn) + // Lock is aquired here to prevent race condition with Close() occuring + // during this background reconnect attempt s.closingMutex.Lock() s.conns = append(s.conns, conn) s.closingMutex.Unlock() From 0f283cadcffd5e878f9889648a5ba57b04a75a37 Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 16:41:37 -0500 Subject: [PATCH 09/11] refactor --- go/stream.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/go/stream.go b/go/stream.go index 6155b82..b25f8fa 100644 --- a/go/stream.go +++ b/go/stream.go @@ -136,17 +136,11 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs if err != nil { return } - go s.monitorConn(conn) - // Lock is aquired here to prevent race condition with Close() occuring - // during this background reconnect attempt - s.closingMutex.Lock() - s.conns = append(s.conns, conn) - s.closingMutex.Unlock() + s.addConn(conn) }() continue } - go s.monitorConn(conn) - s.conns = append(s.conns, conn) + s.addConn(conn) } // Only fail if we couldn't connect to ANY origins @@ -170,6 +164,16 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs return s, nil } +// addConn adds a connection to the monitoring goroutine aquires the closingMutex to append the connection +// to the stream's connection list safely. It uses the closingMutex to prevent a race conditions with Close() +// but is also using this mutex to prevent race conditions with other goroutines appending to conns. +func (s *stream) addConn(conn *wsConn) { + go s.monitorConn(conn) + s.closingMutex.Lock() + s.conns = append(s.conns, conn) + s.closingMutex.Unlock() +} + func (s *stream) pingConn(ctx context.Context, conn *wsConn) { ticker := time.NewTicker(time.Second * 2) defer ticker.Stop() From 2553993aa71d050542b5bf491e1fca5721d2814d Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 17:25:51 -0500 Subject: [PATCH 10/11] remove race from test --- .github/workflows/go-tests.yml | 3 ++- go/stream.go | 16 ++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 188c88c..ba74119 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -23,6 +23,7 @@ jobs: go-version: "1.24" cache-dependency-path: go/go.sum + # Runs test with shuffle and count to expose flaky tests. - name: Run tests working-directory: go - run: go test -race ./... + run: go test -count=10 -shuffle=on ./... diff --git a/go/stream.go b/go/stream.go index b25f8fa..acd1ec5 100644 --- a/go/stream.go +++ b/go/stream.go @@ -136,11 +136,13 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs if err != nil { return } - s.addConn(conn) + go s.monitorConn(conn) + s.conns = append(s.conns, conn) }() continue } - s.addConn(conn) + go s.monitorConn(conn) + s.conns = append(s.conns, conn) } // Only fail if we couldn't connect to ANY origins @@ -164,16 +166,6 @@ func (c *client) newStream(ctx context.Context, httpClient *http.Client, feedIDs return s, nil } -// addConn adds a connection to the monitoring goroutine aquires the closingMutex to append the connection -// to the stream's connection list safely. It uses the closingMutex to prevent a race conditions with Close() -// but is also using this mutex to prevent race conditions with other goroutines appending to conns. -func (s *stream) addConn(conn *wsConn) { - go s.monitorConn(conn) - s.closingMutex.Lock() - s.conns = append(s.conns, conn) - s.closingMutex.Unlock() -} - func (s *stream) pingConn(ctx context.Context, conn *wsConn) { ticker := time.NewTicker(time.Second * 2) defer ticker.Stop() From 0b18e52c91edfeeb66f4f410f93966eb1f838e0e Mon Sep 17 00:00:00 2001 From: Alex Kuznicki Date: Thu, 29 Jan 2026 17:30:30 -0500 Subject: [PATCH 11/11] reduce test count --- .github/workflows/go-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index ba74119..d829aae 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -26,4 +26,4 @@ jobs: # Runs test with shuffle and count to expose flaky tests. - name: Run tests working-directory: go - run: go test -count=10 -shuffle=on ./... + run: go test -count=5 -shuffle=on ./...