chore(build): refresh harness and dependency floor#62
Merged
Conversation
Pull the workspace-resident upstreams forward to their latest tagged
versions:
- darvaza.org/core v0.18.5 -> v0.19.0
- darvaza.org/slog v0.8.1 -> v0.9.0
- darvaza.org/slog/handlers/discard v0.6.2 -> v0.7.0
- darvaza.org/x/config v0.5.0 -> v0.6.0
- darvaza.org/x/container v0.3.1 -> v0.4.0
- darvaza.org/x/fs v0.5.2 -> v0.6.0
- darvaza.org/x/net v0.6.2 -> v0.7.0
- darvaza.org/x/sync v0.3.0 -> v0.4.0
Transitive: golang.org/x/{crypto,net,sys,text} pulled forward to match
the new floor. pkg/nanopb carries no darvaza dependency and is unchanged.
Signed-off-by: Alejandro Mery <amery@apptly.co>
Two clean-ups on the cspell configuration: - Remove the .vscode/cspell.json symlink and point the cSpell VSCode extension at internal/build/cspell.json via the cSpell.import setting in .vscode/settings.json. Keeps a single canonical wordlist on disk and removes the need for a symlinked duplicate. - Prefix every internal/build/cspell.json ignorePaths entry with `**/` so the patterns recurse into subdirectories. The bare basename `nanopb.pb.go` failed to match `pkg/nanopb/nanopb.pb.go` under cspell 10, leaking 32 false positives from the generated protobuf file. Widen the entry to `**/*.pb.go` while we're here. - Drop the leading `$schema` URL; the value is informational only and was the sole non-data line in the file. Signed-off-by: Alejandro Mery <amery@apptly.co>
A `default:` clause makes a switch's no-match handling visible instead of leaving the reader to guess whether the omission was deliberate. Add explicit defaults — each with a one-line comment naming what is kept instead of handled — to the switches in ResponseError.String, HashCache.computeHash, and Session.Send. The arg-less t.Fatalf calls in the protocol and request-handler tests become t.Fatal, the correct form when there is no format string to apply. No behaviour change. Signed-off-by: Alejandro Mery <amery@apptly.co>
Pass -count=1 to `go test` from the per-module `test-<name>` recipe emitted by gen_mk.sh so the cached pass from a previous run cannot mask a regression. Every `make test` now re-executes the package tests rather than reprinting `(cached)`. GOTEST_FLAGS is unchanged, so callers can still layer -race, -run, or -v on top from the command line. Signed-off-by: Alejandro Mery <amery@apptly.co>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughUpgrades build/tooling to Go 1.24, centralizes and tightens lint/spellcheck configs, refactors CLI tool invocation to ChangesBuild System & Code Quality Modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/build/fix_whitespace.sh`:
- Around line 53-56: The use of "chmod --reference" (in the tmp/file replacement
block around tmp, file) is GNU-only and breaks on macOS/BSD; replace it with a
portable fallback: attempt "chmod --reference" first, and if it fails, read the
original file's mode using a portable stat fallback (try "stat -c %a" for GNU,
"stat -f %Lp" for BSD/macOS) into a variable and then call "chmod $mode $tmp"
before mv "$tmp" "$file"; keep references to the existing tmp and file variables
and ensure the fallback path only runs when the --reference invocation fails.
In `@Makefile`:
- Line 60: The prune rule is hardcoding ".tmp" in the FIND_FILES_PRUNE_RULES
variable; change it to reference the TMPDIR make variable so the rule follows
any overridden TMPDIR (e.g., replace occurrences of ".tmp" with $(TMPDIR) or
$${TMPDIR} as appropriate), and ensure TMPDIR has a sensible default earlier in
the Makefile if not already defined; update the FIND_FILES_PRUNE_RULES
definition that currently contains ".tmp" so it uses the TMPDIR symbol instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90c7e4d3-29b7-4ac1-b454-fa19310d2406
⛔ Files ignored due to path filters (2)
pkg/generator/go.sumis excluded by!**/*.sumpkg/nanorpc/go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
.vscode/cspell.json.vscode/settings.jsonMakefileinternal/build/cspell.jsoninternal/build/fix_whitespace.shinternal/build/gen_mk.shinternal/build/languagetool.cfginternal/build/make_codecov.shinternal/build/markdownlint.jsoninternal/build/revive-v1.14.tomlinternal/build/revive.tomlpkg/generator/go.modpkg/nanorpc/client/session.gopkg/nanorpc/errors.gopkg/nanorpc/go.modpkg/nanorpc/hashcache.gopkg/nanorpc/nanorpc_protocol_test.gopkg/nanorpc/server/handler_request_test.go
💤 Files with no reviewable changes (1)
- .vscode/cspell.json
| endif | ||
|
|
||
| FIND_FILES_PRUNE_RULES ?= -name vendor -o -name .git -o -name node_modules | ||
| FIND_FILES_PRUNE_RULES ?= -name vendor -o -name .git -o -name node_modules -o -name .tmp |
There was a problem hiding this comment.
Tie prune rules to TMPDIR instead of hardcoding .tmp.
Line 60 hardcodes .tmp, so overriding TMPDIR to another in-repo directory can cause temp artifacts to be linted/formatted unexpectedly.
Suggested patch
+TMPDIR_BASENAME ?= $(notdir $(TMPDIR))
-FIND_FILES_PRUNE_RULES ?= -name vendor -o -name .git -o -name node_modules -o -name .tmp
+FIND_FILES_PRUNE_RULES ?= -name vendor -o -name .git -o -name node_modules -o -name $(TMPDIR_BASENAME)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` at line 60, The prune rule is hardcoding ".tmp" in the
FIND_FILES_PRUNE_RULES variable; change it to reference the TMPDIR make variable
so the rule follows any overridden TMPDIR (e.g., replace occurrences of ".tmp"
with $(TMPDIR) or $${TMPDIR} as appropriate), and ensure TMPDIR has a sensible
default earlier in the Makefile if not already defined; update the
FIND_FILES_PRUNE_RULES definition that currently contains ".tmp" so it uses the
TMPDIR symbol instead.
The top-level Makefile and the scripts under internal/build/ are shared across the darvaza.org and protomcp.org repos. The nanorpc copy had drifted; this brings it back into line with core. Toolchain pins move to tier-aware versions: - golangci-lint: v2.8.0 (Go 1.24) / v2.11.4 (Go 1.25+) - revive: v1.14.0 (Go 1.24) / v1.15.0 (Go 1.25+) Other substantive changes: - `pnpx` gives way to `pnpm dlx` throughout (pnpx is deprecated); DLX falls back to `true` when pnpm is absent - `.tmp` joins the find-prune list - fix_whitespace.sh strips a UTF-8 BOM and trailing blank lines, via new get_bytes/quote_arg/filter_file helpers - make_codecov.sh writes through a `~` sidecar and renames only when content changes, preserving codecov.sh's mtime - markdownlint code-block line length is raised to 120; MD060 is enabled - gen_mk.sh fixes a `.PHONY` typo (merge-coverage → merged-coverage) - the codecov target's recipe loses its inline make_codecov.sh call; the script is produced solely through the `$(COVERAGE_DIR)/codecov.sh` prerequisite For revive, `pkg/nanorpc/utils` trips the default bad-name list. A rename sits outside this commit's scope, so revive.toml's `[rule.package-naming]` enables only `skipDefaultBadNameCheck`; convention, top-level, and stdlib-collision checks stay active. The new internal/build/revive-v1.14.toml expresses the same intent for the Go 1.24 tier through `var-naming`'s `skipPackageNameChecks` option, since `package-naming` did not exist as a separate rule until v1.15.0. The Makefile selects between the two configs via `REVIVE_CONF_FILE`, matching the pattern used by darvaza.org/x. The GitHub Actions workflows under .github/workflows/ pick up the matching drift correction from darvaza.org/x: - A top-level `permissions: contents: read` block on every workflow, locking the default GITHUB_TOKEN to read-only. - actions/checkout pinned to v6 across all five workflows; actions/setup-node to v6 in renovate.yml. - The Go matrix shifts to ['1.24', '1.25', '1.26'] in build.yml and test.yml; codecov.yml and race.yml move their single-version pin to '1.26'. - build.yml's matrix variable is renamed from `go` to `go-version` for consistency with test.yml. The only retained nanorpc-local divergences from core are gen_index.sh's `GROUPS=pkg` and cspell.json's project vocabulary. Signed-off-by: Alejandro Mery <amery@apptly.co>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Around line 21-22: Update the GitHub Actions steps that use actions/checkout
and actions/setup-go to pin to specific commit SHAs instead of tags (replace
uses: actions/checkout@v6 and uses: actions/setup-go@v6 with the corresponding
full SHA references) and add persist-credentials: false to the checkout step to
disable credential persistence; ensure the checkout step includes the
persist-credentials: false field and the setup-go step uses the exact commit SHA
for the referenced action.
In @.github/workflows/codecov.yml:
- Around line 18-21: Replace mutable action tags actions/checkout@v6 and
actions/setup-go@v6 with their corresponding pinned commit SHAs and disable
checkout credential persistence: update the checkout step (referenced by
actions/checkout@v6) to include persist-credentials: false and pin to the
checkout action's digest SHA, and pin the setup-go step (actions/setup-go@v6) to
its digest SHA as well so the workflow no longer relies on mutable tags and the
CODECOV_TOKEN is not persisted in the runner credentials.
In @.github/workflows/race.yml:
- Around line 18-21: Update the workflow to harden the two actions: replace the
actions/checkout@v6 reference with the full 40-character commit SHA and add the
checkout step input persist-credentials: false (since no authenticated Git
operations are needed), and replace actions/setup-go@v6 with its full
40-character commit SHA as well; ensure both action uses keep the same step
names (actions/checkout and actions/setup-go) so reviewers can easily locate and
verify the pinned SHAs and the persist-credentials setting.
In @.github/workflows/renovate.yml:
- Around line 20-21: Replace the mutable action pins with immutable commit SHAs
and disable persisted checkout credentials: update the uses entries for
actions/checkout and actions/setup-node (the two lines using actions/checkout@v6
and actions/setup-node@v6) to use their full commit SHA refs, and add
persist-credentials: false under the actions/checkout step so the checkout does
not retain authentication tokens.
In @.github/workflows/test.yml:
- Around line 21-24: Replace the mutable action pins for actions/checkout@v6 and
actions/setup-go@v6 with their corresponding full commit SHAs and add
persist-credentials: false to the checkout step; locate the checkout step (uses:
actions/checkout) and update it to include persist-credentials: false, and
replace both uses: actions/checkout@v6 and uses: actions/setup-go@v6 with the
exact commit SHAs (from the actions repositories) to ensure immutability and
supply-chain safety.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37a83671-d6e1-4498-ba8a-0faf6b168f40
📒 Files selected for processing (13)
.github/workflows/build.yml.github/workflows/codecov.yml.github/workflows/race.yml.github/workflows/renovate.yml.github/workflows/test.ymlMakefileinternal/build/fix_whitespace.shinternal/build/gen_mk.shinternal/build/languagetool.cfginternal/build/make_codecov.shinternal/build/markdownlint.jsoninternal/build/revive-v1.14.tomlinternal/build/revive.toml
✅ Files skipped from review due to trivial changes (3)
- internal/build/languagetool.cfg
- internal/build/revive-v1.14.toml
- internal/build/markdownlint.json
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/build/revive.toml
- internal/build/make_codecov.sh
- internal/build/gen_mk.sh
- internal/build/fix_whitespace.sh
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
1 participant
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.
Summary
Refresh the build harness from
darvaza.org/coreafter drift, raisethe toolchain and dependency floors, and apply the source-side
adjustments the new revive tier asks for.
Toolchain and dependency floor
1.23.0 → 1.24.0inpkg/nanorpc/go.modandpkg/generator/go.mod.pkg/nanopbcarries no darvaza dependencyand is unchanged.
v2.3.0(single tier) →v2.8.0(Go 1.24) /v2.11.4(Go 1.25+).v1.7(single tier) →v1.14.0(Go 1.24) /v1.15.0(Go 1.25+). The Makefile selects the tier-aware config via
REVIVE_CONF_FILE, matching the pattern used indarvaza.org/x.core v0.18.5 → v0.19.0,slog v0.8.1 → v0.9.0,slog/handlers/discard v0.6.2 → v0.7.0,x/config v0.5.0 → v0.6.0,x/container v0.3.1 → v0.4.0,x/fs v0.5.2 → v0.6.0,x/net v0.6.2 → v0.7.0,x/sync v0.3.0 → v0.4.0. Transitivegolang.org/x/{crypto,net,sys,text}pulled forward to match.See chore(deps): bump darvaza.org/x, slog, and core to latest
releases.
Build harness and CI workflows
Drift correction on
Makefileandinternal/build/. Substantivechanges:
pnpxis deprecated. The Makefile now resolves aDLXvariableto
pnpm dlx, with atruefallback when pnpm is absent. Allfour pnpx callsites (markdownlint, languagetool, cspell,
shellcheck) follow.
.tmpjoins the find-prune list.fix_whitespace.shstrips a UTF-8 BOM and trailing blank linesvia new
get_bytes/quote_arg/filter_filehelpers.make_codecov.shwrites through a~sidecar and renames onlywhen content changes, preserving
codecov.sh's mtime so make'sprerequisite tracking stays stable.
enabled.
gen_mk.sh:.PHONY: merge-coveragetypo corrected tomerged-coverage.make_codecov.shcall; the script is produced solely through the
$(COVERAGE_DIR)/codecov.shprerequisite chain.For revive,
pkg/nanorpc/utilstrips the default bad-name list. Arename is outside scope. The new
internal/build/revive.toml(v1.15+) enables only
package-naming.skipDefaultBadNameCheck. Thenew
internal/build/revive-v1.14.toml(Go 1.24 tier) expresses thesame intent via
var-naming'sskipPackageNameChecksoption, sincepackage-namingdid not exist as a separate rule until v1.15.0.Convention, top-level, and stdlib-collision checks stay active in
both tiers.
The only retained nanorpc-local divergences from core are
gen_index.sh'sGROUPS=pkgandcspell.json's projectvocabulary.
The GitHub Actions workflows under
.github/workflows/pick upthe matching drift correction from
darvaza.org/x:permissions: contents: readblock on everyworkflow, locking the default
GITHUB_TOKENto read-only.actions/checkoutpinned to v6 across all five workflows;actions/setup-nodeto v6 inrenovate.yml.['1.24', '1.25', '1.26']inbuild.ymlandtest.yml;codecov.ymlandrace.ymlmovetheir single-version pin to
'1.26'.build.yml's matrix variable is renamed fromgotogo-versionfor consistency withtest.yml.See chore(build): sync build harness and CI workflows and
build(mk): force -count=1 on the generated test target.
cspell config
.vscode/cspell.jsonsymlink. Point the cSpell VS Codeextension at
internal/build/cspell.jsonvia thecSpell.importsetting. Single canonical wordlist on disk.
ignorePathsentry with**/so the patternsrecurse into subdirectories. The bare basename
nanopb.pb.gofailed to match
pkg/nanopb/nanopb.pb.gounder cspell 10, leaking32 false positives. Widen the entry to
**/*.pb.go.$schemaURL; it was the sole non-data line.See refactor(cspell): drop vscode symlink and broaden ignore
globs.
Source-side adjustments
Triggered by the new revive tier:
default:clauses on the switches inResponseError.String,HashCache.computeHash, andSession.Send, each with a one-line comment naming what is keptinstead of handled.
t.Fatalfbecomest.Fatalin the protocol andrequest-handler tests (the correct form when there is no format
string to apply).
No behaviour change.
See refactor: make switch no-ops explicit; t.Fatal where unused.
Notes for reviewers
make testre-executes every package every run;-count=1isforced through
gen_mk.shso a cached pass from a previous runcannot mask a regression.
GOTEST_FLAGSis unchanged, so callerscan still layer
-race,-run, or-von top.(
revive.tomlfor v1.15+,revive-v1.14.tomlfor the Go 1.24tier). Both files exist permanently; the Makefile picks the right
one for the active Go version.
Test plan
makeis green — tidy across all four modules, cspell,shellcheck, build.
make testis green acrossgenerator,nanopb, andnanorpc.(
revive-v1.14.toml).revive.toml).floor.
Summary by CodeRabbit