ci: make four checks actually able to fail
Why
Four checks in the pipeline either could not run or could not fail. Each is fixed here, and the new race job found a real data race in the signer.
Changes
go-tss never ran on dependency bumps. The job triggered only on changes under bifrost/tss/go-tss/**, and the package is excluded from unit-tests via TEST_PATHS. A tss-lib-private bump in go.mod therefore ran no TSS tests at all. go.mod and go.sum are now in the job's changes paths.
Nothing ran the race detector over bifrost. Only test-go-tss used --race. Adds test-race-bifrost and a unit-tests-race-bifrost job, scoped to ./bifrost/... minus go-tss, with a 20m test timeout. The job runs in long-test, not test — see the note below.
Coverage was null on every pipeline. Root cause is not the regex: the full per-function go tool cover -func dump pushed the job log past GitLab's 4MB collection cap, so the total: line the coverage: regex reads was never collected. Verified against a truncated unit-tests trace, which ends mid-line with Job's log exceeded limit of 4194304 bytes. The target now prints only the total line.
semgrep could never fail. The job ended in || true on the deprecated returntocorp/semgrep-agent:v1 image, fetching rules over the network. It now uses the maintained pinned image and the in-repo rule under semgrep/ with --error. --no-git-ignore is deliberate: when git fails inside the image, semgrep's target discovery finds 0 files, reports "Findings: 0", and exits 0. That reproduces the exact silent pass this change removes. Artifacts now upload when: always, since the report matters most when the job fails.
Also adds a govulncheck job. It is allow_failure: true with a dated comment because it cannot pass today (32 reachable vulnerabilities across 18 modules as of 2026-09-14, including libp2p and btcd). Clearing that baseline is a separate piece of work; the comment says to drop allow_failure once it is green.
Signer race fix
pipeline.Wait() polled len() on the semaphore channels with a 1s sleep. Channel length establishes no happens-before with the signing goroutines' writes, so anything read after Wait() could observe stale state. The race detector reports 40 races in bifrost/signer, all rooted in pipeline_test.go reading mock state after Wait() while SpawnSignings goroutines wrote it.
Wait() now blocks on a sync.WaitGroup. Done is deferred first so it runs after the semaphore release, preserving the existing ordering. The only production caller (sign.go) runs Wait() and SpawnSignings sequentially in one goroutine, so reuse of the WaitGroup is safe. Net diff is negative.
ConstantsProvider.EnsureConstants read requestHeight outside constantsLock while getConstantsFromThorchain wrote it under the lock (raised in review). requestHeight and the cached churnInterval are written together, so they are now read together under the lock, and the lock is released before any fetch. TestEnsureConstants_Concurrent covers it: 3 races before, 0 after.
The new race job also caught a test-only race in bifrost/observer: the XMR spent-refs callback runs on the import goroutine, and attestation_spent_refs_committed_test.go read its result after a time.Sleep while each subtest rebound the callback field under the previous subtest's live goroutine. The subtests now hand the result back over a channel. 3 races before, 0 after (-cpu 1,2,4 -count=20).
It also caught a flaky deadline in bifrost/pkg/chainclients/evm: TestSignTxWithAllowanceFlow failed on context deadline exceeded against the in-process httptest server, because all twelve chain configs in that suite set HTTPRequestTimeout: time.Second — not a safe budget for a localhost round trip under -race on a shared runner. Raised to 10s; no test asserts on timeout behaviour.
Verification
| Check | Result |
|---|---|
bifrost/signer under -race, before |
FAIL, 40 races, 90s |
bifrost/signer under -race, after |
PASS, 0 races, 23s |
Full ./bifrost/... under -race |
15.6m locally, sharing CPU with another job |
| semgrep against the tree | 0 findings, 1413 targets scanned, exit 0 |
| semgrep against a positive sample | 1 finding, exit 1 |
glab ci lint and trunk check |
clean on all changed files |
First pipeline should show
unit-testscoverage is a percentage, not null- semgrep log reports roughly 1400 targets scanned, not 0, and the SAST artifact uploads
unit-tests-race-bifrostgreen in thelong-teststagegovulncheckyellow with the vulnerability listing
https://claude.ai/code/session_01HAKyLUixQhaWRzdHvvKJBf
Second push: stage placement and one flaky test
The first pipeline failed unit-tests, lint and unit-tests-race-bifrost. None of it was the code under test — it was CPU starvation on the shared Finland runner, which was also carrying an 85-minute test-simulation from a develop pipeline at the same time.
| Job | This MR, first pipeline | develop baseline |
|---|---|---|
unit-tests |
1372s, failed | 285-425s, green |
lint |
3607s, hit the 1h timeout | 727-1123s, green |
The unit-tests failures are all context deadline exceeded and dial tcp ... i/o timeout against the tests' own httptest servers on localhost, in the EVM, ETH, BTC signer and observer suites. That is a starved runner, not a regression.
A 35-minute race-detector job does not belong in the test stage, where it competes with unit tests that talk to localhost under a 1s HTTP timeout and a lint job with a hard 1h cap. unit-tests-race-bifrost now runs in long-test with the other heavy jobs, with an explicit 1h timeout.
The one genuine failure inside the race job was TestScanBlocksProcessesTxs, which slept a fixed 2s and then asserted the scanner had made progress. Under -race on a loaded runner that budget expired before the first block was scanned. It now polls for the condition with a 30s deadline, so it passes as soon as the work is done and fails only if the work never happens. Verified with -count=5 under -race.
Rebased onto develop.
Third push: two more races the new job caught
Both found by unit-tests-race-bifrost itself, which is the point of adding it.
bifrost/p2p. NodeGaterTestSuite.TestPeriodicRefresh read gater.allowedPeers directly while the goroutine started by Start() replaced the map. NodeGater guards that field with g.mu everywhere in production code; only the test reached around it. The same two assertions were also sleep-then-assert, 50ms and 150ms against a 100ms refresh interval. Both now poll under g.mu.RLock() with a 5s deadline.
bifrost/pkg/chainclients/evm. EVMSuite.SetUpTest starts an httptest server whose handler closes over s.thorKeys, and TearDownTest never closed it, so those handler goroutines outlived their test and raced the next SetUpTest's write of that field. TearDownTest now closes the server; Close waits for outstanding requests, which supplies the happens-before.
Worth noting: the EVM race does not reproduce under -race locally on arm64, and its failure output was invisible in the first pipeline because that job's log was itself truncated at the 4MB cap. Both fixes verified under -race with -count=2/-count=3, and the full ./bifrost/... suite runs clean under -race locally (44 packages, 0 races).
Five other bifrost test files leak an httptest server the same way (thorclient, solana, ethereum, and two unstuck_test.go). They are deliberately left alone: none of them races today, and closing a server that a test left a polling client attached to would hang Close() — a worse failure than the one being prevented.