Repository navigation
sdk%feat(lint): add clang-{format,tidy} 20, instrument with lint_clang.py, validate against cert-{c,cpp}-coding-standards and cpp-security-and-quality suites, fetch per-platform CodeQL - #111
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title names the main change: adding clang-format and clang-tidy 20 linting, a lint_clang.py driver, CERT and security-and-quality CodeQL suites, and per-platform CodeQL fetching. The changeset mat… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The description directly relates to the changes. It explains the planned C/C++ support, CodeQL platform bundles, lint changes, testing, and motivation. |
Full details: Title check
-
Explanation
The title names the main change: adding clang-format and clang-tidy 20 linting, a lint_clang.py driver, CERT and security-and-quality CodeQL suites, and per-platform CodeQL fetching. The changeset matches each of these. The title is long (192 characters, 16 words) and packs many items into one line, but it is related to the main change and is not vague or off-topic.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
|
Note This pull request has no conflicts! 🎊 🎉 🎊 |
github-actions
Bot
added
the
topic:Build/CI
|
✅ Final review complete — no blockers (commit 901e651) · triage: normal |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
thepastaclaw
reviewed
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the supplied findings against head 0119f58 and the base-to-head source changes. No blocking defect was confirmed; the diagnostic-filtering regression-test suggestion is valid and in scope. Current CI checks, including Lint, pass, and the lint log confirms the new C/C++ gates executed; local validation was static only.
🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — This cross-cutting lint and build-tooling change adds substantial orchestration and platform-specific CodeQL packaging without changing any critical runtime surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `maint/lint/lint_codeql.py`:
- [SUGGESTION] maint/lint/lint_codeql.py:239-245: Add regression fixtures for diagnostic filtering and suppressions
The new ownership filter controls which diagnostics contribute to the failure count, including findings from the existing Rust security queries. Neither this filtering nor the new TOML suppression loader has accompanying regression tests. The passing lint run verifies integration on the current sources, but does not establish that an unsuppressed owned-file diagnostic still fails the gate. Add small CSV/TOML fixture tests covering an owned finding being counted, an external-header finding being excluded, exact path-and-message suppression without suppressing a different file or message, and malformed CSV rows or suppression records being rejected. These helpers can be tested without invoking CodeQL.
kwvg
marked this pull request as ready for review
November 2, 2026 16:24
`git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
thepastaclaw
reviewed
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently inspected the complete base-to-head diff at 901e651 and found no verified blocking defect; the regression-coverage suggestion remains valid. Current CI reports successful lint, stable-platform builds, and cross-builds, with MSRV and additional platform checks still pending and no failed checks reported. Validation was static only; no builds or tests were executed locally.
🟡 1 suggestion(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — This is a cross-cutting lint and build-tooling change with platform-specific CodeQL packaging and new analysis orchestration, but it does not change a critical runtime surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `maint/lint/lint_codeql.py`:
- [SUGGESTION] maint/lint/lint_codeql.py:239-245: Add regression fixtures for diagnostic filtering and suppressions
(existing thread: https://github.com/dashpay/base-sdk/pull/111#discussion_r4444226145)
The ownership filter and per-language suppressions determine which diagnostics can fail the lint gate, but neither `_print_csv_diagnostics` nor `_load_suppressions` has regression coverage in the current tree. Passing analysis does not prove that an owned, unsuppressed violation survives filtering or that a suppression matches only its exact path/message pair. In particular, the two configured suppressions target `__builtin_available` in the macOS-only branch of `contrib/probes/hello.c`, while the CI lint job runs on Ubuntu. Add lightweight CSV/TOML fixtures covering an owned unsuppressed result, an unowned dependency result, an exact suppression, path/message near-matches, newline normalization, and malformed rows or suppression entries; assert both emitted diagnostics and the returned count. These tests can exercise the Python helpers without invoking CodeQL.
4 of 6 tasks
kwvg
added a commit
that referenced
this pull request
Nov 3, 2026
* [e8ce915] pkc%test(bls): add the reference implementation as a submodule (pasta) * [76704ec] sdk%build(nix): add `corpora.nix` with CMake, Boost and GMP (pasta) * [4feb192] pkc%test(bls): add programs that write the BLS corpora (pasta) * [5e0c21a] sdk%ci(gha): build the corpus generators weekly (pasta) Description: ````` ## Motivation The BLS corpora in `pkgs/pkc/corpus` are written from the reference implementation, but the programs that wrote them are not in the repository. This pull request adds them, with `regen.py` to write every corpus again from a pinned release, so a corpus can be checked or extended later. It is split out of #101, which adds one more program on top of it. ## Additional Information | Description | Notes | | ----------- | ----- | | Add the reference implementation as a submodule | `pkgs/pkc/contrib/dash`, pinned at v24.0.0-rc.5 (8163290aa0), shallow and with `update = none`, so a clone, `git submodule update` and CI checkouts skip it. Only `regen.py` checks it out. Bazel, CodeQL and the crate package skip it, as its tree carries build files of its own. | | Add programs that write the BLS corpora | Each `gen_*.cpp` links the release's BLS library (dashbls, with relic), plus a few of the release's own sources where a corpus needs them, and prints one corpus. `regen.py` extracts the release's `src`, builds the library, compiles and runs every program, and writes the corpora only when all of them succeed. The nine corpora come out byte-identical, so this changes no corpus. `pkgs/pkc/contrib/README.md` has the recipe. | | Add autotools and GMP to the dev shell | `contrib/nix/shell/dev.nix` gains autoconf, automake, libtool, pkg-config and GMP for `regen.py`. The CI shell is unchanged. | Bazel has no build for the library, and the release's copy of the library builds with autotools only, as the reference implementation stopped tracking relic's CMake configuration template. So `regen.py` runs that build, which needs a POSIX shell, and GMP is required. When these programs were added, the corpora were also checked equal at v23.1.8 (728f505583). The C++ here is as it was in #101 and predates the clang-format style in #111. As discussed in #101, this branch is open for the C++ lint and static analysis commits. Whichever of this and #111 lands second will adapt. ## Breaking Changes None. No library code changes. ## How Has This Been Tested? - `regen.py` writes the nine corpora byte-identical to the committed ones, with GCC 15 and with clang 21 building both the library and the programs, from a fresh clone that fetches the submodule and from an existing build. - A failed library build, a submodule checked out at another commit and an invalid option each stop the run with nothing written. - Only Linux x86_64 was run. macOS and Windows are untested, and the dev shell change has not been run in a Nix shell. - `cargo test -p dash-pkc --all-features` passes, and the lint suite is clean with the submodule checked out. - CI (`just test lint fuzz::test`, CodeQL included) is green on each commit. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional tests - [x] I have made corresponding changes to the documentation - [ ] I did not use agentic assistance (i.e. LLMs, AI) except as [disclosed](#agent-disclosure) and can answer questions, comments and concerns without said assistance. I assume authorship of the code submitted. - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ### Agent disclosure Code, commit messages and this description were generated with Claude Code using Claude Opus 5.5, running autonomously as an agent on behalf of the author. The agent also built the pinned release's BLS library and wrote the corpora with it. The change was reviewed by a Claude Code agent using Opus 5.5. 🤖 Generated with [Claude Code](https://claude.com/claude-code) `````
PastaPastaPasta
added a commit
to PastaPastaPasta/base-sdk
that referenced
this pull request
Nov 4, 2026
Takes dashpay#100 dashpay#88 dashpay#108 dashpay#107 dashpay#111 dashpay#110 dashpay#112 dashpay#115 dashpay#114 dashpay#101. ChainSetup keeps the DEC-1043 policy fields; the BS11 BLS wrappers, i64 TxOut values and MAX_SER_SIZE payload bound stay for Core parity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
As described in base-sdk#101, we will be introducing C/C++ sources for generating corpora (and later for FFIs for our libraries),
Additional Information
This pull request makes
just linta very expensive step as it will be running CodeQL thrice, reformulating workflows to parallelise this step and readjustingjustverbs to makejust combmore resource-economic will be addressed in a future pull request to avoid reworking CI until FFIs have been committed in tree.During local platform validation we identified that aarch64 Linux CodeQL broke again and now that all-platform bundles have been deprecated by GitHub (source) we're going back to managing CodeQL ourselves to repair aarch64 Linux though the fixes are a lot less invasive than prior iterations so the maintenance burden will be limited.
How Has This Been Tested?
Checklist
Agent disclosure
Code generated with Claude Code using Opus 5.5, medium effort. Reviewed with Codex using GPT-6.1-Sol, medium effort as adversarial review and Claude Code using Fable 5.1, medium effort.