CI: Gate on C23 compilers; move older compilers to canaries - #159
Conversation
…ing canaries; add docs/CI_STRATEGY.md; add gcc-14 target to gauntlet Dockerfile
📝 WalkthroughWalkthroughWorkflow narrows gating compilers to gcc-14 and clang-20, adds non-blocking canary compilers, and sets minimal workflow permissions. New CI strategy doc added. Dockerfile reorganized around Ubuntu 24.04 base-noble with a shared /test-compiler.sh runner and a new gcc-14 stage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Gate as Job: gating (gcc-14, clang-20)
participant Can as Job: canary (gcc-12, gcc-13, clang-18, clang-19)
Dev->>GH: Push/PR triggers workflow
GH->>Gate: Start gating matrix (parallel)
Note over Gate: Checkout → Setup Buildx → Docker build → Test\nUpload logs to build/
Gate-->>GH: Results (required)
GH->>Can: Start canary matrix (needs: gating), continue-on-error
Note over Can: Checkout → Setup Buildx → Docker build → Test\nUpload logs to build/ (retention 7d)
Can-->>GH: Results (informational)
GH-->>Dev: Report statuses — gating required, canaries non-blocking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
…: contents: read)
…o base; fix AS casing
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tools/gauntlet/Dockerfile.gauntlet (4)
20-43: Normalize Dockerfile syntax and slim images.
- Use uppercase AS to satisfy linters.
- Add --no-install-recommends and clean apt lists in all RUN apt-get blocks.
Example (apply similarly to gcc-12/13, clang-18/19/20 stages):
-FROM base as gcc-12 +FROM base AS gcc-12 -RUN apt-get update && apt-get install -y \ +RUN apt-get update && apt-get install -y --no-install-recommends \ software-properties-common \ wget \ gnupg && \ add-apt-repository ppa:ubuntu-toolchain-r/test -y && \ apt-get update && \ - apt-get install -y gcc-12 g++-12 + apt-get install -y gcc-12 g++-12 && \ + rm -rf /var/lib/apt/lists/*
92-133: Avoid redefining the final default stage solely for script; inherit from base instead.If you keep a default image, derive it from clang-20 but do not duplicate script/CMD there after moving them to base. Otherwise remove the trailing default stage to avoid confusion.
-# Default to clang-20 for standalone usage -FROM clang-20 - -# Simple test script that works with any compiler -... long script creation ... -WORKDIR /workspace -CMD ["/test-compiler.sh"] +# Default to clang-20 for standalone usage (inherits script/CMD from base) +FROM clang-20
1-132: Optional: HEALTHCHECK and non-root.
- Add a simple HEALTHCHECK (e.g., $CC --version) to appease CKV_DOCKER_2.
- Non-root user improves hardening (CKV_DOCKER_3), but beware mounted volume perms on GitHub runners. If you switch to USER ci, also run docker with --user or adjust ownership of /workspace.
1-92: Move test runner, WORKDIR and CMD into the base stage so all compiler stages inherit them.
The /test-compiler.sh creation plus WORKDIR /workspace and CMD are only present in the clang-20 final stage (lines 96–132); move that RUN (script creation) and the WORKDIR/CMD into the base stage immediately after the package installation (meson) and remove the final-stage block..github/workflows/gauntlet.yml (1)
1-8: Add minimal GITHUB_TOKEN permissions.Harden the workflow per CodeQL suggestion.
name: GNU CRY GAUNTLET on: push: branches: [ main ] pull_request: branches: [ main ] + +permissions: + contents: read
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/gauntlet.yml(1 hunks)docs/CI_STRATEGY.md(1 hunks)tools/gauntlet/Dockerfile.gauntlet(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/CI_STRATEGY.md
4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
6-6: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
36-36: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
42-42: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
45-45: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
46-46: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
50-50: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
54-54: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🪛 Checkov (3.2.334)
tools/gauntlet/Dockerfile.gauntlet
[low] 1-132: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-132: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 GitHub Actions: GNU CRY GAUNTLET
tools/gauntlet/Dockerfile.gauntlet
[error] 46-52: Docker build failed. Command 'apt-get update && apt-get install -y software-properties-common wget gnupg && add-apt-repository ppa:ubuntu-toolchain-r/test -y && apt-get update && apt-get install -y gcc-14 g++-14' exited with code 100: E: Unable to locate package gcc-14. (tools/gauntlet/Dockerfile.gauntlet:46-52)
[warning] 21-21: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 21)
[warning] 33-33: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 33)
[warning] 45-45: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 45)
[warning] 57-57: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 57)
[warning] 69-69: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 69)
[warning] 81-81: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 81)
[warning] 2-2: FromAsCasing: 'as' and 'FROM' keywords casing do not match (line 2)
🪛 GitHub Check: CodeQL
.github/workflows/gauntlet.yml
[warning] 37-62: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
🪛 YAMLlint (1.37.1)
.github/workflows/gauntlet.yml
[error] 18-18: wrong indentation: expected 6 but found 4
(indentation)
[error] 46-46: wrong indentation: expected 6 but found 4
(indentation)
| - name: Build ${{ matrix.compiler }} Docker image | ||
| run: | | ||
| echo "🔥 Building ${{ matrix.compiler }} Docker image..." | ||
| echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." | ||
| docker build --target ${{ matrix.compiler }} -t gauntlet-${{ matrix.compiler }}:latest -f tools/gauntlet/Dockerfile.gauntlet . | ||
|
|
||
| - name: Run ${{ matrix.compiler }} GAUNTLET | ||
| run: | | ||
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET..." | ||
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET (gating)..." | ||
| docker run --rm -v "$PWD":/workspace -w /workspace gauntlet-${{ matrix.compiler }}:latest | ||
|
|
||
| - name: Upload build logs on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: gauntlet-logs-${{ matrix.compiler }} | ||
| path: | | ||
| build/ | ||
| retention-days: 7 No newline at end of file | ||
| path: build/ | ||
| retention-days: 7 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Artifacts on failure are good; consider always saving Meson logs.
Optionally upload build/meson-logs even on success for gating to aid regressions; keep canary as-is.
🤖 Prompt for AI Agents
.github/workflows/gauntlet.yml lines 20-35: the workflow only uploads build
artifacts on failure; add an additional artifact upload step that always runs
(no if: failure()) to persist Meson logs even on successful runs so regressions
can be analyzed. Add a step after the GAUNTLET run that uses
actions/upload-artifact@v4 to upload build/meson-logs (and/or build/), giving it
a clear name like gauntlet-meson-logs-${{ matrix.compiler }} and retention-days:
7; keep the existing failure-only upload for broader logs if desired.
| ### Gating (C23 focus) | ||
| - `gcc-14` — first broadly usable GCC with C23 support | ||
| - `clang-20` — latest LLVM with strong C23 + clang-tidy 20 | ||
|
|
||
| Rationale: These represent the C23 target surface we care about. They block the PR when they fail. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Align doc with implementation detail: gcc-14 stage uses Ubuntu 24.04 base.
Since gcc-14 is fetched from Ubuntu 24.04 in Docker (to fix CI), note that in the doc for reproducibility.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🤖 Prompt for AI Agents
In docs/CI_STRATEGY.md around lines 13 to 18, update the Gating (C23 focus)
section to mention that the gcc-14 CI stage uses an Ubuntu 24.04 base image
(since gcc-14 is fetched from Ubuntu 24.04 in Docker) so readers can reproduce
the environment; add a concise parenthetical or sentence after thegcc-14 bullet
indicating the Ubuntu 24.04 base and, if helpful, reference the Dockerfile or CI
job name where this is defined.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
tools/gauntlet/Dockerfile.gauntlet (6)
22-31: gcc-12 stage OK; mirror the base cleanup.Keep image sizes consistent across stages.
-RUN apt-get update && apt-get install -y \ +RUN apt-get update && apt-get install -y --no-install-recommends \ @@ - apt-get install -y gcc-12 g++-12 + apt-get install -y gcc-12 g++-12 && rm -rf /var/lib/apt/lists/*
34-43: gcc-13 stage OK; mirror the base cleanup.-RUN apt-get update && apt-get install -y \ +RUN apt-get update && apt-get install -y --no-install-recommends \ @@ - apt-get install -y gcc-13 g++-13 + apt-get install -y gcc-13 g++-13 && rm -rf /var/lib/apt/lists/*
69-79: Avoid piping remote scripts to bash; add minimal verification or repo setup.wget -O - ... | bash is a supply-chain risk. Prefer apt repo steps or verify checksum before execution.
- wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 18 all && \ - apt-get install -y clang-18 clang++-18 clang-tidy-18 + wget -O /tmp/llvm.sh https://apt.llvm.org/llvm.sh && \ + chmod +x /tmp/llvm.sh && \ + /tmp/llvm.sh 18 all && \ + apt-get install -y --no-install-recommends clang-18 clang++-18 clang-tidy-18 && \ + rm -rf /var/lib/apt/lists/* /tmp/llvm.shIf you want, I can provide the explicit apt-keyless repository commands for apt.llvm.org.
81-91: Repeat the llvm.sh hardening for clang-19.- wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 19 all && \ - apt-get install -y clang-19 clang++-19 clang-tidy-19 + wget -O /tmp/llvm.sh https://apt.llvm.org/llvm.sh && chmod +x /tmp/llvm.sh && \ + /tmp/llvm.sh 19 all && \ + apt-get install -y --no-install-recommends clang-19 clang++-19 clang-tidy-19 && \ + rm -rf /var/lib/apt/lists/* /tmp/llvm.sh
93-103: Repeat the llvm.sh hardening for clang-20.- wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 20 all && \ - apt-get install -y clang-20 clang++-20 clang-tidy-20 + wget -O /tmp/llvm.sh https://apt.llvm.org/llvm.sh && chmod +x /tmp/llvm.sh && \ + /tmp/llvm.sh 20 all && \ + apt-get install -y --no-install-recommends clang-20 clang++-20 clang-tidy-20 && \ + rm -rf /var/lib/apt/lists/* /tmp/llvm.sh
104-145: Run as non-root and add a basic HEALTHCHECK.Addresses CKV_DOCKER_2 and CKV_DOCKER_3 and improves runtime safety.
FROM clang-20 +RUN groupadd -g 1000 app && useradd -m -u 1000 -g app app @@ - chmod +x /test-compiler.sh + chmod +x /test-compiler.sh && chown -R app:app /workspace /test-compiler.sh +USER app +HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 CMD [ -x /test-compiler.sh ] || exit 1 @@ -WORKDIR /workspace +WORKDIR /workspace CMD ["/test-compiler.sh"]
♻️ Duplicate comments (5)
.github/workflows/gauntlet.yml (5)
14-19: Add job timeout and concurrency to avoid hung builds and cancel superseded runs.gating: runs-on: ubuntu-latest name: C23 Gating Compilers + timeout-minutes: 45 + concurrency: + group: gauntlet-gating-${{ github.ref }} + cancel-in-progress: trueApply similar to canary with a shorter timeout.
38-46: Set canary timeout and concurrency; keep continue-on-error.canary: runs-on: ubuntu-latest name: Compat Canaries (non-blocking) continue-on-error: true needs: [gating] + timeout-minutes: 25 + concurrency: + group: gauntlet-canary-${{ github.ref }} + cancel-in-progress: true
31-36: Optionally always upload Meson logs for triage, not only on failure.- name: Upload build logs on failure if: failure() uses: actions/upload-artifact@v4 with: name: gauntlet-logs-${{ matrix.compiler }} path: build/ retention-days: 7 + - name: Upload Meson logs (always) + if: always() + uses: actions/upload-artifact@v4 + with: + name: gauntlet-meson-logs-${{ matrix.compiler }} + path: build/meson-logs/ + retention-days: 7Mirror in canary if desired.
38-49: Fix canary steps indentation (yamllint error).- steps: - - uses: actions/checkout@v4 - - uses: docker/setup-buildx-action@v3 + steps: + - uses: actions/checkout@v4 + - uses: docker/setup-buildx-action@v3
20-22: Fix steps indentation (yamllint error).Indent list items under steps by two spaces.
- steps: - - uses: actions/checkout@v4 - - uses: docker/setup-buildx-action@v3 + steps: + - uses: actions/checkout@v4 + - uses: docker/setup-buildx-action@v3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/gauntlet.yml(2 hunks)tools/gauntlet/Dockerfile.gauntlet(7 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/gauntlet.yml
[error] 20-20: wrong indentation: expected 6 but found 4
(indentation)
[error] 48-48: wrong indentation: expected 6 but found 4
(indentation)
🪛 Checkov (3.2.334)
tools/gauntlet/Dockerfile.gauntlet
[low] 1-144: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-144: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🔇 Additional comments (4)
tools/gauntlet/Dockerfile.gauntlet (2)
16-16: libroaring-dev addition looks good.
62-66: gcc-14 gating stage is correct and aligns with CI strategy..github/workflows/gauntlet.yml (2)
2-3: Good: explicit minimal GITHUB_TOKEN permissions.
12-18: Matrix change to gcc-14/clang-20 aligns with C23 gating.
| - name: Build ${{ matrix.compiler }} Docker image | ||
| run: | | ||
| echo "🔥 Building ${{ matrix.compiler }} Docker image..." | ||
| echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." | ||
| docker build --target ${{ matrix.compiler }} -t gauntlet-${{ matrix.compiler }}:latest -f tools/gauntlet/Dockerfile.gauntlet . | ||
|
|
||
| - name: Run ${{ matrix.compiler }} GAUNTLET | ||
| run: | | ||
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET..." | ||
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET (gating)..." | ||
| docker run --rm -v "$PWD":/workspace -w /workspace gauntlet-${{ matrix.compiler }}:latest |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider passing explicit platform and enabling BuildKit cache to speed Docker builds.
- - name: Build ${{ matrix.compiler }} Docker image
+ - name: Build ${{ matrix.compiler }} Docker image
run: |
- echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..."
- docker build --target ${{ matrix.compiler }} -t gauntlet-${{ matrix.compiler }}:latest -f tools/gauntlet/Dockerfile.gauntlet .
+ echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..."
+ docker build --pull --target ${{ matrix.compiler }} \
+ --tag gauntlet-${{ matrix.compiler }}:latest \
+ --file tools/gauntlet/Dockerfile.gauntlet \
+ --build-arg BUILDKIT_INLINE_CACHE=1 .Optionally add cache-to/cache-from with docker/build-push-action for stronger caching.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build ${{ matrix.compiler }} Docker image | |
| run: | | |
| echo "🔥 Building ${{ matrix.compiler }} Docker image..." | |
| echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." | |
| docker build --target ${{ matrix.compiler }} -t gauntlet-${{ matrix.compiler }}:latest -f tools/gauntlet/Dockerfile.gauntlet . | |
| - name: Run ${{ matrix.compiler }} GAUNTLET | |
| run: | | |
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET..." | |
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET (gating)..." | |
| docker run --rm -v "$PWD":/workspace -w /workspace gauntlet-${{ matrix.compiler }}:latest | |
| - name: Build ${{ matrix.compiler }} Docker image | |
| run: | | |
| echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." | |
| docker build --pull --target ${{ matrix.compiler }} \ | |
| --tag gauntlet-${{ matrix.compiler }}:latest \ | |
| --file tools/gauntlet/Dockerfile.gauntlet \ | |
| --build-arg BUILDKIT_INLINE_CACHE=1 . | |
| - name: Run ${{ matrix.compiler }} GAUNTLET | |
| run: | | |
| echo "🔫 Running ${{ matrix.compiler }} GAUNTLET (gating)..." | |
| docker run --rm -v "$PWD":/workspace -w /workspace gauntlet-${{ matrix.compiler }}:latest |
🤖 Prompt for AI Agents
In .github/workflows/gauntlet.yml around lines 22 to 29, the Docker build/run
steps don't specify a platform or enable BuildKit/caching which slows CI; update
the build step to pass an explicit platform (e.g. --platform ${{ matrix.platform
}} or --platform linux/amd64) and enable BuildKit for faster builds (e.g. export
DOCKER_BUILDKIT=1 before docker build), and either add basic buildkit cache
flags (like --cache-from and --cache-to when available) or replace the plain
docker build/run with the docker/build-push-action and configure
cache-from/cache-to and restore-keys to enable layer caching across runs.
…ists (gating/canary)\n- Dockerfile: --no-install-recommends + apt cache cleanup; move test runner/WORKDIR/CMD to base; remove default final stage; add HEALTHCHECK; keep uppercase AS stages
…onfig; fix fuzz step typo\n- core-quality: install libroaring-dev and pkg-config
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/c_core.yml (1)
32-36: Clarify cppcheck usage and make fuzzing bounded.
- You still run cppcheck (Line 34) even though the gating emphasis moved to clang-tidy; confirm this is intentional and that the target path "src" is correct (other steps target core/). Consider moving cppcheck to the non-gating quality workflow to avoid double static analysis in gates.
- The fuzz step changed to 60 seconds. Verify this duration preserves coverage for typical regressions and add a job-level timeout to avoid runaway hangs.
Example tweaks:
jobs: build-test: runs-on: ubuntu-latest + timeout-minutes: 30 @@ - - run: python tools/run_fuzz.py 60 + - name: Fuzz smoke (60s) + run: python tools/run_fuzz.py 60.github/workflows/core-quality.yml (1)
121-127: Artifact path fix LGTM; keep logs flowing.coverage.html path change is correct. Consider also uploading build/meson-logs for easier triage.
Example:
- name: Upload coverage report uses: actions/upload-artifact@v4 if: always() with: name: coverage-report path: build-coverage/coverage.html + + - name: Upload Meson logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: meson-logs-quality + path: build-**/meson-logs/ + retention-days: 7
♻️ Duplicate comments (2)
.github/workflows/gauntlet.yml (2)
12-19: Add job timeouts and concurrency to prevent hangs and cancel superseded runs.Applies to both gating and canary. This was suggested previously; reiterating here.
jobs: gating: runs-on: ubuntu-latest name: C23 Gating Compilers + timeout-minutes: 45 + concurrency: + group: gauntlet-gating-${{ github.ref }} + cancel-in-progress: true @@ canary: runs-on: ubuntu-latest name: Compat Canaries (non-blocking) continue-on-error: true needs: [gating] + timeout-minutes: 25 + concurrency: + group: gauntlet-canary-${{ github.ref }} + cancel-in-progress: trueAlso applies to: 38-47
20-36: Speed up and stabilize Docker builds.Pull latest bases and enable BuildKit caching to reduce rebuilds.
- - name: Build ${{ matrix.compiler }} Docker image + - name: Build ${{ matrix.compiler }} Docker image run: | - echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." - docker build --target ${{ matrix.compiler }} -t gauntlet-${{ matrix.compiler }}:latest -f tools/gauntlet/Dockerfile.gauntlet . + echo "🔥 Building ${{ matrix.compiler }} Docker image (gating)..." + export DOCKER_BUILDKIT=1 + docker build --pull \ + --target ${{ matrix.compiler }} \ + --tag gauntlet-${{ matrix.compiler }}:latest \ + --file tools/gauntlet/Dockerfile.gauntlet .
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/c_core.yml(2 hunks).github/workflows/core-quality.yml(2 hunks).github/workflows/gauntlet.yml(2 hunks)tools/gauntlet/Dockerfile.gauntlet(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-13T03:14:24.159Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:14:24.159Z
Learning: Run ./tools/docker-clang-tidy.sh and ensure no new clang-tidy warnings before PRs
Applied to files:
.github/workflows/c_core.yml
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Run build, tests, clang-tidy (via tools/docker-clang-tidy.sh), and gauntlet locally before proposing changes; keep changes zero-warnings and formatted
Applied to files:
.github/workflows/c_core.yml
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Use Meson/Ninja for build and tests; run lint and gauntlet locally; keep diffs minimal and pass CI with zero new clang-tidy warnings
Applied to files:
.github/workflows/c_core.ymltools/gauntlet/Dockerfile.gauntlet.github/workflows/gauntlet.yml
🪛 Checkov (3.2.334)
tools/gauntlet/Dockerfile.gauntlet
[low] 1-126: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🔇 Additional comments (6)
.github/workflows/c_core.yml (1)
13-19: Deps update LGTM; matches C23 toolchain needs.Adding pkg-config and libroaring-dev is appropriate and consistent with the rest of the PR. No issues spotted.
.github/workflows/core-quality.yml (1)
22-23: Deps update LGTM.Adding libroaring-dev and pkg-config aligns the quality workflow with build requirements. Good.
.github/workflows/gauntlet.yml (2)
2-3: Good hardening: explicit minimal permissions.permissions: contents: read is a best practice. ✔️
12-19: Gating matrix choice LGTM.gcc-14 and clang-20 reflect real C23 support while keeping fail-fast disabled across the matrix. ✔️
tools/gauntlet/Dockerfile.gauntlet (2)
8-20: Base image deps and cleanup LGTM.Using --no-install-recommends and cleaning apt lists keeps layers lean; libroaring-dev addition aligns with CI changes. ✔️
44-52: GCC-12 stage OK; minimal and cleaned.Using toolchain PPA with cache cleanup looks good.
| - name: Upload build logs on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: gauntlet-logs-${{ matrix.compiler }} | ||
| path: build/ | ||
| retention-days: 7 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Always persist Meson logs for triage.
Keep the failure-only artifact, but also upload meson-logs even on success.
- name: Upload build logs on failure
if: failure()
uses: actions/upload-artifact@v4
with:
name: gauntlet-logs-${{ matrix.compiler }}
path: build/
retention-days: 7
+ - name: Upload Meson logs (always)
+ if: always()
+ uses: actions/upload-artifact@v4
+ with:
+ name: meson-logs-${{ matrix.compiler }}
+ path: build/meson-logs/
+ retention-days: 7Repeat for canary block.
Also applies to: 58-64
| canary: | ||
| runs-on: ubuntu-latest | ||
| name: Compat Canaries (non-blocking) | ||
| continue-on-error: true | ||
| needs: [gating] | ||
| strategy: | ||
| matrix: | ||
| compiler: [gcc-12, gcc-13, clang-18, clang-19] | ||
| fail-fast: false | ||
| steps: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Let canaries run even if gating fails.
To keep portability signals, run canaries regardless of gating result.
canary:
runs-on: ubuntu-latest
name: Compat Canaries (non-blocking)
continue-on-error: true
- needs: [gating]
+ needs: [gating]
+ if: always()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| canary: | |
| runs-on: ubuntu-latest | |
| name: Compat Canaries (non-blocking) | |
| continue-on-error: true | |
| needs: [gating] | |
| strategy: | |
| matrix: | |
| compiler: [gcc-12, gcc-13, clang-18, clang-19] | |
| fail-fast: false | |
| steps: | |
| canary: | |
| runs-on: ubuntu-latest | |
| name: Compat Canaries (non-blocking) | |
| continue-on-error: true | |
| needs: [gating] | |
| if: always() | |
| strategy: | |
| matrix: | |
| compiler: [gcc-12, gcc-13, clang-18, clang-19] | |
| fail-fast: false | |
| steps: |
🤖 Prompt for AI Agents
.github/workflows/gauntlet.yml around lines 38 to 47: the canary job is blocked
by the gating job because it uses needs: [gating], so it won't run if gating
fails; to ensure canaries always run for portability signals, keep the needs for
ordering but add a job-level condition of if: always() to force execution
regardless of gating result (alternatively remove the needs dependency if
ordering isn’t required).
| # Base image with common dependencies | ||
| FROM ubuntu:22.04 as base | ||
| FROM ubuntu:22.04 AS base | ||
|
|
||
| ENV DEBIAN_FRONTEND=noninteractive | ||
| ENV TZ=UTC | ||
|
|
||
| # Install base dependencies | ||
| RUN apt-get update && apt-get install -y \ | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| build-essential \ | ||
| ninja-build \ | ||
| python3 \ | ||
| python3-pip \ | ||
| pkg-config \ | ||
| libsodium-dev \ | ||
| libgit2-dev \ | ||
| libroaring-dev \ | ||
| git \ | ||
| curl \ | ||
| meson | ||
| meson && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Shared test runner and defaults for all compiler stages | ||
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | ||
| echo 'set -e' >> /test-compiler.sh && \ | ||
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | ||
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | ||
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | ||
| echo 'rm -rf build' >> /test-compiler.sh && \ | ||
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | ||
| echo ' echo "PASS"' >> /test-compiler.sh && \ | ||
| echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ | ||
| echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ | ||
| echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ | ||
| chmod +x /test-compiler.sh | ||
| WORKDIR /workspace | ||
| CMD ["/test-compiler.sh"] | ||
| HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Run as non-root to appease policy checks (optional).
Create a non-root user and switch after tool install; CI mounts will still work for builds.
@@
WORKDIR /workspace
-CMD ["/test-compiler.sh"]
-HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1"
+RUN useradd -m -u 1000 builder
+USER builder
+CMD ["/test-compiler.sh"]
+HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1"Repeat USER builder in derived stages if overridden.
Also applies to: 84-89, 91-125
🧰 Tools
🪛 Checkov (3.2.334)
[low] 1-126: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
In tools/gauntlet/Dockerfile.gauntlet around lines 1-41, the image runs as root
which can fail policy checks; after installing packages create a non-root user
(e.g., builder), chown /workspace (and any other tool directories) to that user,
and switch to USER builder before the WORKDIR/CMD so CI mounts still work;
ensure any derived stages that override USER also explicitly run USER builder
(repeat the same create/chown/switch pattern in the other stage ranges noted:
lines ~84-89 and ~91-125).
| # Shared test runner and defaults for all compiler stages | ||
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | ||
| echo 'set -e' >> /test-compiler.sh && \ | ||
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | ||
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | ||
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | ||
| echo 'rm -rf build' >> /test-compiler.sh && \ | ||
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | ||
| echo ' echo "PASS"' >> /test-compiler.sh && \ | ||
| echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ | ||
| echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ | ||
| echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ | ||
| chmod +x /test-compiler.sh | ||
| WORKDIR /workspace | ||
| CMD ["/test-compiler.sh"] | ||
| HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1" |
There was a problem hiding this comment.
Critical: test runner does not exit non-zero on failures.
The if/else echos “FAIL_*” but never exits 1, so docker run returns 0 even when setup/build/tests fail. Gate would pass incorrectly.
Apply:
RUN echo '#!/bin/bash' > /test-compiler.sh && \
- echo 'set -e' >> /test-compiler.sh && \
+ echo 'set -euo pipefail' >> /test-compiler.sh && \
@@
- echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \
+ echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \
@@
- echo ' if ninja -C build test; then' >> /test-compiler.sh && \
+ echo ' if ninja -C build test; then' >> /test-compiler.sh && \
@@
- echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; fi' >> /test-compiler.sh && \
- echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \
- echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \
+ echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; exit 1; fi' >> /test-compiler.sh && \
+ echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; exit 1; fi' >> /test-compiler.sh && \
+ echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; exit 1; fi' >> /test-compiler.sh && \
chmod +x /test-compiler.sh📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Shared test runner and defaults for all compiler stages | |
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | |
| echo 'set -e' >> /test-compiler.sh && \ | |
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | |
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | |
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | |
| echo 'rm -rf build' >> /test-compiler.sh && \ | |
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | |
| echo ' echo "PASS"' >> /test-compiler.sh && \ | |
| echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ | |
| echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ | |
| echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ | |
| chmod +x /test-compiler.sh | |
| WORKDIR /workspace | |
| CMD ["/test-compiler.sh"] | |
| HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1" | |
| # Shared test runner and defaults for all compiler stages | |
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | |
| echo 'set -euo pipefail' >> /test-compiler.sh && \ | |
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | |
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | |
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | |
| echo 'rm -rf build' >> /test-compiler.sh && \ | |
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | |
| echo ' echo "PASS"' >> /test-compiler.sh && \ | |
| echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; exit 1; fi' >> /test-compiler.sh && \ | |
| echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; exit 1; fi' >> /test-compiler.sh && \ | |
| echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; exit 1; fi' >> /test-compiler.sh && \ | |
| chmod +x /test-compiler.sh | |
| WORKDIR /workspace | |
| CMD ["/test-compiler.sh"] | |
| HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD bash -lc "$CC --version >/dev/null 2>&1 || exit 1" |
| FROM base AS clang-18 | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| wget \ | ||
| gnupg \ | ||
| lsb-release \ | ||
| software-properties-common && \ | ||
| wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 18 all && \ | ||
| apt-get install -y clang-18 clang++-18 clang-tidy-18 | ||
| apt-get install -y --no-install-recommends clang-18 clang++-18 clang-tidy-18 && rm -rf /var/lib/apt/lists/* | ||
| ENV CC=clang-18 | ||
| ENV CXX=clang++-18 | ||
|
|
||
| # Clang 19 - C23 support image | ||
| FROM base as clang-19 | ||
| RUN apt-get update && apt-get install -y \ | ||
| FROM base AS clang-19 | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| wget \ | ||
| gnupg \ | ||
| lsb-release \ | ||
| software-properties-common && \ | ||
| wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 19 all && \ | ||
| apt-get install -y clang-19 clang++-19 clang-tidy-19 | ||
| apt-get install -y --no-install-recommends clang-19 clang++-19 clang-tidy-19 && rm -rf /var/lib/apt/lists/* | ||
| ENV CC=clang-19 | ||
| ENV CXX=clang++-19 | ||
|
|
||
| # Clang 20 - Latest C23 support image | ||
| FROM base as clang-20 | ||
| RUN apt-get update && apt-get install -y \ | ||
| FROM base AS clang-20 | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| wget \ | ||
| gnupg \ | ||
| lsb-release \ | ||
| software-properties-common && \ | ||
| wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 20 all && \ | ||
| apt-get install -y clang-20 clang++-20 clang-tidy-20 | ||
| apt-get install -y --no-install-recommends clang-20 clang++-20 clang-tidy-20 && rm -rf /var/lib/apt/lists/* | ||
| ENV CC=clang-20 | ||
| ENV CXX=clang++-20 | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clang stages look solid; minor hardening optional.
llvm.sh usage + cleanup is fine. Optionally pin llvm.sh by checksum and add --depth=1 curl/wget to reduce surface.
🤖 Prompt for AI Agents
In tools/gauntlet/Dockerfile.gauntlet around lines 91 to 125, the Dockerfile
currently pipes the remote llvm.sh installer directly into bash which is
flexible but risky; update each clang-N stage to download the llvm.sh script to
a temporary file using a safe downloader (e.g., curl -fsSL), verify its SHA256
checksum against a pinned value, only then execute it with bash -s -- <version>
all, and remove the temp file; this hardens the build and you can also reduce
network surface by using a minimal download invocation (curl -fsSL) instead of
piping directly from wget.
…eeping main's structure and paths
…rt C23 gating job
… apt clean; docs: fix CI_STRATEGY markdown spacing
|
Addressed CodeRabbit suggestions:\n- Dockerfile.gauntlet: uppercase AS, --no-install-recommends + apt clean, HEALTHCHECK, libroaring-dev. Test runner is now in base so all targets inherit it; gcc-14 stage added for gating.\n- docs/CI_STRATEGY.md: spacing fixes for markdownlint.\n\nCI is green on gating + canaries. Please re-run checks and lift changes-requested when satisfied. 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/CI_STRATEGY.md (1)
1-63: Enforce markdownlint + link-check in CI and fix docs errorsRan npx markdownlint-cli2 + lychee — the run reported 2,485 markdownlint issues across docs. docs/CI_STRATEGY.md (lines 1–63) shows MD022 (missing blank lines around headings), MD013 (line-length >80), MD009 (trailing spaces) and fenced-code issues.
- Add a docs-lint workflow that runs: npx -y markdownlint-cli2 "docs/**/*.md" && lychee --no-progress --quiet docs/ (fail CI on markdownlint; whitelist/soft-fail external domains for link checks as needed).
- Fix high-priority failures immediately — start with docs/CI_STRATEGY.md (1–63): add blank lines around headings, wrap long lines to ≤80 chars, remove trailing spaces, ensure fenced code blocks have a language and surrounding blank lines.
- Add a pre-commit hook / GitHub Action to run the same checks locally to prevent regressions.
♻️ Duplicate comments (3)
tools/gauntlet/Dockerfile.gauntlet (1)
24-45: Make the test runner fail fast and return non‑zero on failures.As written, failures echo markers but exit 0, so CI could pass incorrectly.
Apply:
RUN echo '#!/bin/bash' > /test-compiler.sh && \ - echo 'set -e' >> /test-compiler.sh && \ + echo 'set -euo pipefail' >> /test-compiler.sh && \ @@ - echo ' if ninja -C build test; then' >> /test-compiler.sh && \ + echo ' if ninja -C build test; then' >> /test-compiler.sh && \ @@ - echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ - echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ - echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ + echo ' else echo "❌ Tests failed"; echo "FAIL_TEST"; exit 1; fi' >> /test-compiler.sh && \ + echo ' else echo "❌ Build failed"; echo "FAIL_BUILD"; exit 1; fi' >> /test-compiler.sh && \ + echo 'else echo "❌ Meson setup failed"; echo "FAIL_SETUP"; exit 1; fi' >> /test-compiler.sh && \ chmod +x /test-compiler.shdocs/CI_STRATEGY.md (2)
14-15: Document the gcc‑14 base (Ubuntu 24.04) for reproducibility.Call out that the gcc‑14 Docker stage uses a 24.04 base so users can match CI locally.
-- `gcc-14` — first broadly usable GCC with C23 support +- `gcc-14` — first broadly usable GCC with C23 support (runs on Ubuntu 24.04 base in Docker)
3-8: Fix markdownlint issues: add blank lines around headings/lists; remove trailing spaces.Addresses MD022/MD032/MD009 flagged by tooling.
-## Overview -This repository enforces high correctness and portability while centering on modern C23. CI is split into: -- Gating jobs that must pass for merges +## Overview + +This repository enforces high correctness and portability while centering on modern C23. CI is split into: + +- Gating jobs that must pass for merges - Canary jobs that provide early portability signals but do not block - Static analysis (clang-tidy) with a strict baseline - Deterministic Docker builds to ensure local=CI parity ## Compiler Matrix - ### Gating (C23 focus) - + - `gcc-14` — first broadly usable GCC with C23 support - `clang-20` — latest LLVM with strong C23 + clang-tidy 20 Rationale: These represent the C23 target surface we care about. They block the PR when they fail. ### Canaries (non‑blocking) - + - `gcc-12`, `gcc-13` — older GCCs common on LTS distros - `clang-18`, `clang-19` — earlier LLVMs Rationale: Useful signals for downstream consumers. They run with `continue-on-error: true` so they never block merges. ```mermaid flowchart LR @@Static Analysis
clang-tidy20 withquality/.clang-tidy; warnings-as-errors.- Baseline file
tools/baseline_count.txtenforces no regressions.- CI builds a deterministic LLVM 20 environment in Docker.
- CRoaring is installed from apt when available, or built from source in CI to stabilize aarch64 and older runners.
Builds & Tests
- Meson + Ninja builds and runs unit tests.
- Cache artifacts are not pushed; they are derived and may be uploaded only for debugging.
Docker Hygiene
- Images are namespaced and labeled for safe cleanup:
gitmind/ci:clang-20,gitmind/gauntlet:<compiler>; labelcom.gitmind.project=git-mind.- Local helper:
make docker-cleanremoves only project images/containers and builder cache.Time & Cost
- Gating jobs run in parallel and should complete under a few minutes.
- Canaries may be slower; they are non‑blocking and primarily diagnostic.
Future Enhancements
- Add cohesion-report to CI to summarize semantic (advice) changes across merges.
- Nightly long‑matrix (more compilers/OSes) as a separate scheduled workflow.
- Cache Docker layers to reduce cold start times.
Also applies to: 10-18, 19-25, 36-42, 43-47, 48-52, 53-57, 58-63 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: ASSERTIVE **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e3f80ac3f1df60d065e0ee0472c96396456983be and 995e56f6b91cd59ac3e3816300b7d6e66b35689c. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/CI_STRATEGY.md` (1 hunks) * `tools/gauntlet/Dockerfile.gauntlet` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>docs/**</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Keep architecture/dev/ops documentation under docs/ and validate links/TOC with tooling Files: - `docs/CI_STRATEGY.md` </details> </details><details> <summary>🧠 Learnings (4)</summary> <details> <summary>📚 Learning: 2025-09-13T03:13:54.145Z</summary>Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Use Meson/Ninja for build and tests; run lint and gauntlet locally; keep diffs minimal and pass CI with zero new clang-tidy warnings**Applied to files:** - `docs/CI_STRATEGY.md` - `tools/gauntlet/Dockerfile.gauntlet` </details> <details> <summary>📚 Learning: 2025-09-13T03:13:54.145Z</summary>Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to {core,include}/**/*.{c,h} : Formatting: follow .clang-format (LLVM-based, 4 spaces, 80 cols, pointer alignment right); pre-commit enforces clang-format**Applied to files:** - `docs/CI_STRATEGY.md` </details> <details> <summary>📚 Learning: 2025-09-13T03:13:54.145Z</summary>Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Run build, tests, clang-tidy (via tools/docker-clang-tidy.sh), and gauntlet locally before proposing changes; keep changes zero-warnings and formatted**Applied to files:** - `docs/CI_STRATEGY.md` - `tools/gauntlet/Dockerfile.gauntlet` </details> <details> <summary>📚 Learning: 2025-09-13T03:14:24.159Z</summary>Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:14:24.159Z
Learning: Run ./tools/docker-clang-tidy.sh and ensure no new clang-tidy warnings before PRs**Applied to files:** - `tools/gauntlet/Dockerfile.gauntlet` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/CI_STRATEGY.md</summary> 3-3: Headings should be surrounded by blank lines Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) --- 5-5: Lists should be surrounded by blank lines (MD032, blanks-around-lists) --- 13-13: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 20-20: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 37-37: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 44-44: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 49-49: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 54-54: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) --- 59-59: Trailing spaces Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces) </details> </details> <details> <summary>🪛 Checkov (3.2.334)</summary> <details> <summary>tools/gauntlet/Dockerfile.gauntlet</summary> [low] 1-151: Ensure that a user for the container has been created (CKV_DOCKER_3) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)</summary> * GitHub Check: Compat Canaries (non-blocking) (clang-18) * GitHub Check: Compat Canaries (non-blocking) (clang-19) * GitHub Check: Compat Canaries (non-blocking) (gcc-13) * GitHub Check: Compat Canaries (non-blocking) (gcc-12) * GitHub Check: build-test * GitHub Check: Analyze (c-cpp) * GitHub Check: lint-and-sanitize </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| WORKDIR /workspace | ||
| CMD ["/test-compiler.sh"] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Run as non‑root (appeases policy checks, safer defaults).
Create a non‑root user once tools are installed; inherited by derived stages.
WORKDIR /workspace
-CMD ["/test-compiler.sh"]
+RUN useradd -m -u 1000 builder && chown -R builder:builder /workspace
+USER builder
+CMD ["/test-compiler.sh"]Repeat the same USER builder pattern in base-noble:
WORKDIR /workspace
-CMD ["/test-compiler.sh"]
+RUN useradd -m -u 1000 builder && chown -R builder:builder /workspace
+USER builder
+CMD ["/test-compiler.sh"]Also applies to: 144-145
🤖 Prompt for AI Agents
In tools/gauntlet/Dockerfile.gauntlet around lines 46-47 (and similarly at
144-145), the image ends up running as root; create a non-root user after all
tools are installed, chown the /workspace (and any tool directories) to that
user, and set USER to that unprivileged account before WORKDIR and CMD so
derived stages inherit it; apply the same USER creation/chown pattern inside the
base-noble stage as well.
| FROM base AS clang-18 | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| wget \ | ||
| gnupg \ | ||
| lsb-release \ | ||
| software-properties-common && \ | ||
| wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 18 all && \ | ||
| apt-get install -y clang-18 clang++-18 clang-tidy-18 | ||
| apt-get install -y --no-install-recommends clang-18 clang++-18 clang-tidy-18 && \ | ||
| rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden LLVM installer usage; avoid piping remote scripts to shell.
Download llvm.sh with checksum verification, then execute; repeat for 18/19/20.
Example for clang-20:
RUN apt-get update && apt-get install -y --no-install-recommends \
@@
- software-properties-common && \
- wget -O - https://apt.llvm.org/llvm.sh | bash -s -- 20 all && \
+ software-properties-common ca-certificates && \
+ curl -fsSL -o /tmp/llvm.sh https://apt.llvm.org/llvm.sh && \
+ echo "${LLVM_SH_SHA256} /tmp/llvm.sh" | sha256sum -c - && \
+ bash /tmp/llvm.sh 20 all && rm -f /tmp/llvm.sh && \
apt-get install -y --no-install-recommends clang-20 clang++-20 clang-tidy-20 && \
rm -rf /var/lib/apt/lists/*At stage top add:
+ARG LLVM_SH_SHA256=<pin-the-current-sha256>Also applies to: 89-97, 102-110
🤖 Prompt for AI Agents
In tools/gauntlet/Dockerfile.gauntlet around lines 76 to 84 (and similarly at
89-97 and 102-110), the Dockerfile currently pipes the remote llvm.sh installer
directly to bash; replace that with a secure download-and-verify flow: curl or
wget the llvm.sh to a local file, fetch or embed the expected checksum, verify
the checksum (sha256sum) before execution, then run the verified script; repeat
this hardened pattern for clang versions 18, 19 and 20; also add the stage-top
artifacts the reviewer suggested (local script path and checksum variables) so
each stage uses the downloaded-and-verified script rather than piping from the
network.
| # Provide test runner in base-noble too | ||
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | ||
| echo 'set -e' >> /test-compiler.sh && \ | ||
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | ||
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | ||
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | ||
| echo '' >> /test-compiler.sh && \ | ||
| echo '# Clean previous build' >> /test-compiler.sh && \ | ||
| echo 'rm -rf build' >> /test-compiler.sh && \ | ||
| echo '' >> /test-compiler.sh && \ | ||
| echo '# Setup build with current compiler' >> /test-compiler.sh && \ | ||
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | ||
| echo ' ' >> /test-compiler.sh && \ | ||
| echo ' # Build' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | ||
| echo ' ' >> /test-compiler.sh && \ | ||
| echo ' # Run tests' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | ||
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | ||
| echo ' echo "PASS"' >> /test-compiler.sh && \ | ||
| echo ' else' >> /test-compiler.sh && \ | ||
| echo ' echo "❌ Tests failed"' >> /test-compiler.sh && \ | ||
| echo ' echo "FAIL_TEST"' >> /test-compiler.sh && \ | ||
| echo ' fi' >> /test-compiler.sh && \ | ||
| echo ' else' >> /test-compiler.sh && \ | ||
| echo ' echo "❌ Build failed"' >> /test-compiler.sh && \ | ||
| echo ' echo "FAIL_BUILD"' >> /test-compiler.sh && \ | ||
| echo ' fi' >> /test-compiler.sh && \ | ||
| echo 'else' >> /test-compiler.sh && \ | ||
| echo ' echo "❌ Meson setup failed"' >> /test-compiler.sh && \ | ||
| echo ' echo "FAIL_SETUP"' >> /test-compiler.sh && \ | ||
| echo 'fi' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | ||
| echo ' if ninja -C build test; then echo "PASS"; else echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ | ||
| echo ' else echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ | ||
| echo 'else echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ | ||
| chmod +x /test-compiler.sh |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
De‑duplicate the runner; copy from base (also fixes exit code once base is patched).
Avoid drift by reusing the same /test-compiler.sh from the base stage.
-# Provide test runner in base-noble too
-RUN echo '#!/bin/bash' > /test-compiler.sh && \
- echo 'set -e' >> /test-compiler.sh && \
- echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \
- echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \
- echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \
- echo 'rm -rf build' >> /test-compiler.sh && \
- echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \
- echo ' if ninja -C build; then' >> /test-compiler.sh && \
- echo ' if ninja -C build test; then echo "PASS"; else echo "FAIL_TEST"; fi' >> /test-compiler.sh && \
- echo ' else echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \
- echo 'else echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \
- chmod +x /test-compiler.sh
+COPY --from=base /test-compiler.sh /test-compiler.sh
+RUN chmod +x /test-compiler.sh📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Provide test runner in base-noble too | |
| RUN echo '#!/bin/bash' > /test-compiler.sh && \ | |
| echo 'set -e' >> /test-compiler.sh && \ | |
| echo 'echo "🔫 Testing $CC..."' >> /test-compiler.sh && \ | |
| echo 'echo "Compiler: $CC"' >> /test-compiler.sh && \ | |
| echo 'echo "C++ Compiler: $CXX"' >> /test-compiler.sh && \ | |
| echo '' >> /test-compiler.sh && \ | |
| echo '# Clean previous build' >> /test-compiler.sh && \ | |
| echo 'rm -rf build' >> /test-compiler.sh && \ | |
| echo '' >> /test-compiler.sh && \ | |
| echo '# Setup build with current compiler' >> /test-compiler.sh && \ | |
| echo 'if meson setup build --warnlevel=3; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Meson setup successful"' >> /test-compiler.sh && \ | |
| echo ' ' >> /test-compiler.sh && \ | |
| echo ' # Build' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Build successful"' >> /test-compiler.sh && \ | |
| echo ' ' >> /test-compiler.sh && \ | |
| echo ' # Run tests' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build test; then' >> /test-compiler.sh && \ | |
| echo ' echo "✅ Tests passed"' >> /test-compiler.sh && \ | |
| echo ' echo "PASS"' >> /test-compiler.sh && \ | |
| echo ' else' >> /test-compiler.sh && \ | |
| echo ' echo "❌ Tests failed"' >> /test-compiler.sh && \ | |
| echo ' echo "FAIL_TEST"' >> /test-compiler.sh && \ | |
| echo ' fi' >> /test-compiler.sh && \ | |
| echo ' else' >> /test-compiler.sh && \ | |
| echo ' echo "❌ Build failed"' >> /test-compiler.sh && \ | |
| echo ' echo "FAIL_BUILD"' >> /test-compiler.sh && \ | |
| echo ' fi' >> /test-compiler.sh && \ | |
| echo 'else' >> /test-compiler.sh && \ | |
| echo ' echo "❌ Meson setup failed"' >> /test-compiler.sh && \ | |
| echo ' echo "FAIL_SETUP"' >> /test-compiler.sh && \ | |
| echo 'fi' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build; then' >> /test-compiler.sh && \ | |
| echo ' if ninja -C build test; then echo "PASS"; else echo "FAIL_TEST"; fi' >> /test-compiler.sh && \ | |
| echo ' else echo "FAIL_BUILD"; fi' >> /test-compiler.sh && \ | |
| echo 'else echo "FAIL_SETUP"; fi' >> /test-compiler.sh && \ | |
| chmod +x /test-compiler.sh | |
| COPY --from=base /test-compiler.sh /test-compiler.sh | |
| RUN chmod +x /test-compiler.sh |
🤖 Prompt for AI Agents
In tools/gauntlet/Dockerfile.gauntlet around lines 131-143, the Dockerfile
rebuilds /test-compiler.sh via many echo lines causing drift and wrong exit
semantics; instead copy the canonical runner from the base stage and preserve
permissions. Replace the echo block with a single COPY --from=<base-stage-name>
/test-compiler.sh /test-compiler.sh and then run chmod +x /test-compiler.sh (or
ensure the copied file already has executable bit) so the image reuses the exact
runner from the base stage and inherits any future fixes (including correct exit
codes).
CI: Gate on C23 compilers; canaries for older
Summary
Related Issue
Type of Change
Testing
./tools/docker-build.shand./tools/docker-test.sh— all unit tests passChecklist
Summary by CodeRabbit
Chores
Documentation