Skip to content

Commit f08303c

Browse files
dzarlaxdzarlax
authored
Add manual race detector CI policy (#179)
* Add manual race detector CI policy * Prevent race dispatch image publishing * Harden manual race workflow
1 parent 5fafdee commit f08303c

3 files changed

Lines changed: 207 additions & 6 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ on:
1515
description: "Build ARM64 image"
1616
type: boolean
1717
default: false
18+
run_race:
19+
description: "Run Go race detector tests"
20+
type: boolean
21+
default: false
1822

1923
env:
2024
REGISTRY: ghcr.io
@@ -45,18 +49,36 @@ jobs:
4549
# services into this job; the build job's purpose is catching
4650
# logic regressions in seconds, not standing up a stack.
4751
#
48-
# CGO_ENABLED stays at 0 (matches the build above). `-race`
49-
# would catch goroutine data-race regressions but requires
50-
# CGO=1, so we skip it; the test set today is mostly
51-
# math-kernel pure functions where -race adds little.
52+
# CGO_ENABLED stays at 0 (matches the build above). Use the
53+
# manual Race Detector job when reviewing concurrency-sensitive
54+
# changes; it runs with CGO=1 because Go's -race requires CGO.
5255
- name: Test
5356
run: CGO_ENABLED=0 go test ./...
5457

58+
race:
59+
name: Race Detector
60+
runs-on: ubuntu-latest
61+
if: ${{ github.event_name == 'workflow_dispatch' && inputs.run_race }}
62+
permissions:
63+
contents: read
64+
steps:
65+
- uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5
66+
with:
67+
persist-credentials: false
68+
69+
- uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6
70+
with:
71+
go-version-file: go.mod
72+
cache: true
73+
74+
- name: Test with race detector
75+
run: CGO_ENABLED=1 go test -race ./...
76+
5577
docker-amd64:
5678
name: Docker Build & Push (amd64)
5779
needs: build
5880
runs-on: ubuntu-latest
59-
if: ${{ github.ref == 'refs/heads/main' && (github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && inputs.build_amd64)) }}
81+
if: ${{ github.ref == 'refs/heads/main' && (github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && inputs.build_amd64 && !inputs.run_race)) }}
6082
permissions:
6183
contents: read
6284
packages: write
@@ -97,7 +119,7 @@ jobs:
97119
name: Docker Build & Push (arm64)
98120
needs: build
99121
runs-on: ubuntu-latest
100-
if: ${{ github.event_name == 'workflow_dispatch' && inputs.build_arm64 }}
122+
if: ${{ github.ref == 'refs/heads/main' && github.event_name == 'workflow_dispatch' && inputs.build_arm64 && !inputs.run_race }}
101123
permissions:
102124
contents: read
103125
packages: write

‎CONTRIBUTING.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ Dependencies are managed through normal Go module resolution. Review dependency
2525

2626
DB-backed integration tests skip when no Postgres connection is configured. To run them, provide libpq environment variables or `READINESS_TEST_DSN`. Tests create throwaway schemas and drop them during cleanup.
2727

28+
### CI and race detector policy
29+
30+
The default required-looking CI job is `Build, Vet & Test`. Keep that job name stable unless repository branch protection is updated at the same time.
31+
32+
Default PR and push CI uses `CGO_ENABLED=0` for build, vet, and tests, matching the production binary. Race detector tests are available as a manual GitHub Actions run: start the `CI` workflow with `run_race=true`. Race-only dispatches do not publish Docker images, even when the image build inputs are left at their defaults. Use this before merging changes that touch tenant management, scheduler goroutines, async AI generation, recompute coordination, Telegram webhook dispatch, or notification loops.
33+
34+
For local checks on a machine with a CGO toolchain:
35+
36+
```bash
37+
CGO_ENABLED=1 go test -race ./...
38+
```
39+
40+
Branch protection is managed in GitHub repository settings, not in this tree. As of 2026-06-06, the GitHub API reports `main` as not protected. Enabling required checks is a separate admin decision; if enabled, require the `Build, Vet & Test` check first and only require `Race Detector` after deciding that every PR should pay the CGO/race runtime cost.
41+
2842
## Pull Request Guidelines
2943

3044
- Keep behavioral changes focused and explain the data contract they affect.

‎docs/ai-plans/2026-06-06-issue-152-ci-race-detector-branch-protection-policy.html‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Implementation Plan: Issue #152 CI Race Detector and Branch Protection Policy</title>
6+
<meta name="viewport" content="width=device-width, initial-scale=1">
7+
<style>
8+
body { margin: 0; font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; background: #f6f7f9; color: #1f2937; line-height: 1.5; }
9+
main { max-width: 1100px; margin: 0 auto; padding: 32px; }
10+
section { background: #fff; border: 1px solid #e5e7eb; border-radius: 8px; padding: 20px; margin: 16px 0; }
11+
h1, h2 { line-height: 1.2; margin-top: 0; }
12+
code { background: #f3f4f6; padding: 2px 5px; border-radius: 4px; }
13+
.badge { display: inline-block; padding: 4px 10px; border-radius: 999px; background: #dcfce7; color: #166534; font-size: 12px; font-weight: 700; }
14+
.risk { border-left: 4px solid #dc2626; }
15+
.approval { border-left: 4px solid #d97706; }
16+
</style>
17+
</head>
18+
<body>
19+
<main>
20+
<h1>Implementation Plan: Issue #152 CI Race Detector and Branch Protection Policy</h1>
21+
<p class="badge">Approved and implemented</p>
22+
23+
<section>
24+
<h2>Task Summary</h2>
25+
<p>Record a clear policy for race-detector CI and branch protection, then implement only the parts that are justified now.</p>
26+
</section>
27+
28+
<section>
29+
<h2>Current Behavior</h2>
30+
<ul>
31+
<li><code>.github/workflows/ci.yml</code> has one required-looking build job named <code>Build, Vet &amp; Test</code>.</li>
32+
<li>The job runs <code>CGO_ENABLED=0 go build</code>, <code>go vet</code>, and <code>go test</code>.</li>
33+
<li>The workflow comment explicitly says <code>-race</code> is skipped because it requires <code>CGO=1</code> and the current test set is mostly pure math-kernel code.</li>
34+
<li>The repo contains concurrency-sensitive code and tests, for example <code>internal/storage/recompute.go</code> and <code>internal/tenants/manager.go</code>.</li>
35+
<li>Branch protection state is a GitHub repository setting, not represented in the working tree.</li>
36+
</ul>
37+
</section>
38+
39+
<section>
40+
<h2>Desired Behavior</h2>
41+
<ul>
42+
<li>A documented policy says when <code>go test -race</code> is required, optional, or deferred.</li>
43+
<li>If race CI is added, it is a separate job with clear CGO expectations and does not slow the default pure test loop unnecessarily.</li>
44+
<li>Branch protection decision is explicit: either require the existing CI check, or document why it remains manual for now.</li>
45+
<li>This issue stays separate from product guardrail tests and no-vendor migration.</li>
46+
</ul>
47+
</section>
48+
49+
<section>
50+
<h2>Assumptions And Unknowns</h2>
51+
<ul>
52+
<li>Assumption: the default fast CI job should remain pure and easy to understand.</li>
53+
<li>Assumption: race detector is most valuable for PRs touching scheduler, tenant manager, async AI generation, recompute, webhook dispatch, or notification goroutines.</li>
54+
<li>Unknown: whether GitHub branch protection is already configured outside the repository.</li>
55+
<li>Unknown: whether the user wants enforcement now or just a recorded revisit policy.</li>
56+
</ul>
57+
</section>
58+
59+
<section>
60+
<h2>Files Likely To Change</h2>
61+
<ul>
62+
<li><code>.github/workflows/ci.yml</code> - add an optional/manual or path-scoped race job only if approved.</li>
63+
<li><code>CONTRIBUTING.md</code> - document local <code>go test -race ./...</code> guidance and CI policy.</li>
64+
<li><code>README.md</code> or <code>docs/ARCHITECTURE.md</code> - only if branch protection policy belongs in operator docs.</li>
65+
<li>GitHub repository settings - if branch protection is approved, apply through <code>gh api</code> after confirming exact required check names.</li>
66+
</ul>
67+
</section>
68+
69+
<section>
70+
<h2>Implementation Steps</h2>
71+
<ol>
72+
<li>Inspect current branch protection via GitHub API before changing settings.</li>
73+
<li>Choose one of three race-detector policies: defer with revisit criteria, manual workflow-only job, or PR job for all branches.</li>
74+
<li>If implementing race CI, add a separate <code>Race Detector</code> job using <code>CGO_ENABLED=1 go test -race ./...</code>.</li>
75+
<li>Keep the existing <code>Build, Vet &amp; Test</code> job name stable unless branch protection is updated at the same time.</li>
76+
<li>Document local developer guidance and revisit criteria.</li>
77+
<li>If branch protection is approved, update GitHub settings and verify required status checks match actual workflow job names.</li>
78+
</ol>
79+
</section>
80+
81+
<section>
82+
<h2>Impact</h2>
83+
<ul>
84+
<li><strong>CI:</strong> possible extra job duration if race detector is enabled.</li>
85+
<li><strong>Repository process:</strong> branch protection could block direct merges/pushes when CI fails.</li>
86+
<li><strong>Runtime:</strong> no production code changes expected.</li>
87+
<li><strong>Permissions:</strong> branch protection changes require GitHub admin capability and explicit user approval.</li>
88+
</ul>
89+
</section>
90+
91+
<section>
92+
<h2>Test Plan</h2>
93+
<ul>
94+
<li><code>go test ./...</code></li>
95+
<li><code>go vet ./...</code></li>
96+
<li>If race job is added: <code>CGO_ENABLED=1 go test -race ./...</code> locally or in CI.</li>
97+
<li>If branch protection is changed: verify via <code>gh api repos/Dzarlax-AI/health_dashboard/branches/main/protection</code>.</li>
98+
<li>After PR: confirm workflow job names match any required status checks.</li>
99+
</ul>
100+
</section>
101+
102+
<section class="risk">
103+
<h2>Risks And Edge Cases</h2>
104+
<ul>
105+
<li>Race detector can be slow and may need CGO toolchain behavior that differs from the production <code>CGO_ENABLED=0</code> build.</li>
106+
<li>Branch protection misconfiguration can block urgent fixes or require admin intervention.</li>
107+
<li>Changing CI policy at the same time as no-vendor migration can make failures harder to attribute; coordinate with issue #170.</li>
108+
</ul>
109+
</section>
110+
111+
<section>
112+
<h2>Rollback Plan</h2>
113+
<p>Remove the race job or switch it back to manual-only. For branch protection, restore the previous protection JSON or remove the newly required check after confirming with the user.</p>
114+
</section>
115+
116+
<section>
117+
<h2>Open Questions</h2>
118+
<ul>
119+
<li>Should race detector be required on every PR, manual-only, or deferred with explicit triggers?</li>
120+
<li>Do we want branch protection on <code>main</code> now?</li>
121+
<li>If issue #170 is approved, should this policy be implemented after no-vendor CI settles?</li>
122+
</ul>
123+
</section>
124+
125+
<section class="approval">
126+
<h2>Approval Gate</h2>
127+
<p>Approved by the user on 2026-06-06. Branch protection settings were inspected but not changed, because settings mutation requires additional explicit approval.</p>
128+
</section>
129+
130+
<section>
131+
<h2>Implementation Result</h2>
132+
<ul>
133+
<li>Kept the default <code>Build, Vet &amp; Test</code> job name stable.</li>
134+
<li>Added a manual-only <code>Race Detector</code> job behind <code>workflow_dispatch</code> input <code>run_race=true</code>.</li>
135+
<li>Race detector job runs <code>CGO_ENABLED=1 go test -race ./...</code> and is not required on normal PR or push runs.</li>
136+
<li>Guarded Docker publish jobs so race-only dispatches do not push images when <code>run_race=true</code>, and so arm64 image publishing is limited to <code>main</code>.</li>
137+
<li>Hardened the manual race job with <code>permissions: contents: read</code>, <code>persist-credentials: false</code>, and SHA-pinned checkout/setup-go actions.</li>
138+
<li>Documented local and CI race-detector guidance in <code>CONTRIBUTING.md</code>.</li>
139+
<li>Recorded current branch protection state: GitHub API returned <code>Branch not protected</code> for <code>main</code> on 2026-06-06.</li>
140+
</ul>
141+
</section>
142+
143+
<section>
144+
<h2>Verification Run</h2>
145+
<ul>
146+
<li><code>go test ./...</code></li>
147+
<li><code>go vet ./...</code></li>
148+
<li><code>CGO_ENABLED=1 go test -race ./...</code> on Windows was blocked before tests by missing <code>gcc</code> in <code>PATH</code>; the manual GitHub Actions job is the Ubuntu validation path.</li>
149+
<li>Manual GitHub Actions run <code>27062004434</code> passed with <code>run_race=true</code>, including the <code>Race Detector</code> job on Ubuntu.</li>
150+
<li>Manual GitHub Actions run <code>27062164558</code> passed with <code>build_amd64=true</code>, <code>build_arm64=true</code>, and <code>run_race=true</code>; race detector passed and both Docker publish jobs were skipped.</li>
151+
<li><code>gh api repos/Dzarlax-AI/health_dashboard/branches/main/protection</code> returned <code>404 Branch not protected</code>.</li>
152+
</ul>
153+
</section>
154+
155+
<section>
156+
<h2>Known Limitations</h2>
157+
<ul>
158+
<li>Race detector is intentionally manual-only; it will not block PRs unless branch protection and workflow policy are changed later.</li>
159+
<li>Local race-detector runs require a working CGO C compiler. This Windows workspace did not have <code>gcc</code> available.</li>
160+
<li>Branch protection remains unchanged to avoid silently imposing repository-admin policy from a code PR.</li>
161+
</ul>
162+
</section>
163+
</main>
164+
</body>
165+
</html>

0 commit comments

Comments
 (0)