Conversation
|
The latest Buf updates on your PR. Results from workflow Protobuf / buf-build (pull_request).
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughA new GitHub Actions workflow is added for Protobuf processing with buf-action, protobuf configuration is updated from DEFAULT to STANDARD linting rules, and multiple proto files undergo formatting standardization including import reordering and annotation bracket spacing adjustments. Documentation is simplified to direct users to package-specific guidelines. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/proto.yml:
- Around line 16-19: Update the workflow-level permissions in the GitHub Actions
YAML to use least privilege by changing the permissions key value from
"contents: write" to "contents: read" while keeping "pull-requests: write"
as-is; ensure no other jobs depend on repository write access (the existing
"npm-publish" job already overrides permissions with "contents: read" and
"id-token: write"), so replace the top-level permissions block's contents entry
accordingly.
- Line 30: The workflow currently disables Buf breaking detection by setting
"breaking: false" in the buf CI step; update the buf action configuration (the
step using "uses: bufbuild/buf-action@v1") to remove or change the "breaking:
false" setting so breaking checks run on pull requests (either delete the
"breaking" key or set it to true/omit it to restore default behavior).
- Around line 7-14: Add a paths filter to the push trigger so the workflow only
runs for proto changes or when the workflow file changes: under the existing
push: block (containing branches: - main and tags: - 'v*') add paths: -
"proto/**" and - ".github/workflows/proto.yml"; repeat the same addition for the
second push block controlling canary publishes (the block around the canary
publish logic) so that branch pushes are skipped unless proto files or the
workflow file changed while tag pushes continue to run.
In `@proto/buf.md`:
- Line 7: Update the sentence "TypeScript definitions are published to npm as
[`@initia/miniwasm-proto`](https://www.npmjs.com/package/@initia/miniwasm-proto)
on every tagged release (`v*`)" to also document that the CI workflow publishes
canary builds from the main branch; explicitly state that canary builds are
published from main in addition to tagged releases and add a short note about
the canary version naming/semantics so consumers know those versions exist and
how they're produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07d8a330-f726-4f0a-9970-159a5f46243d
📒 Files selected for processing (12)
.github/workflows/proto.ymlproto/buf.gen.es.yamlproto/buf.mdproto/buf.yamlproto/miniwasm/tokenfactory/v1/authority_metadata.protoproto/miniwasm/tokenfactory/v1/genesis.protoproto/miniwasm/tokenfactory/v1/params.protoproto/miniwasm/tokenfactory/v1/query.protoproto/miniwasm/wasmextension/v1/tx.protoproto/miniwasm/wasmextension/v1/types.protoproto/npm/.gitignoreproto/npm/package.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/proto.yml (1)
50-51: Pinprotoc-gen-esto an exact version for reproducible publishes.Line 51 uses
@^2, allowing any 2.x release (currently up to 2.11.0). Pinning to a specific version (e.g.,@2.11.0) ensures consistent builds across runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/proto.yml around lines 50 - 51, Replace the loose semver pin for the protoc plugin so builds are reproducible: change the npm install target "@bufbuild/protoc-gen-es@^2" to an exact version (for example "@bufbuild/protoc-gen-es@2.11.0") in the "Install protoc-gen-es" step so the workflow always installs the same protoc-gen-es release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/proto.yml:
- Around line 12-14: The pull_request trigger only lists paths: "proto/**" so
edits to the workflow itself won't trigger CI; update the pull_request.paths
configuration (the pull_request and paths keys and the "proto/**" entry) to also
include workflow changes (e.g., add a pattern for GitHub workflows such as
.github/workflows/**) or remove path filtering so PRs that modify workflows will
run the job.
---
Nitpick comments:
In @.github/workflows/proto.yml:
- Around line 50-51: Replace the loose semver pin for the protoc plugin so
builds are reproducible: change the npm install target
"@bufbuild/protoc-gen-es@^2" to an exact version (for example
"@bufbuild/protoc-gen-es@2.11.0") in the "Install protoc-gen-es" step so the
workflow always installs the same protoc-gen-es release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c3c3967-62a0-4127-b395-1177086f6bfd
📒 Files selected for processing (1)
.github/workflows/proto.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/proto.yml (1)
17-19: 🛠️ Refactor suggestion | 🟠 MajorChange
contents: writetocontents: readfor least privilege.Neither job requires repository write access. The
buf-buildjob only runs lint/format checks, andnpm-publishalready overrides withcontents: readat line 39.🔒 Proposed fix
permissions: - contents: write + contents: read pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/proto.yml around lines 17 - 19, Update the workflow permissions mapping to follow least privilege: change the top-level permissions entry "contents: write" to "contents: read" so jobs like buf-build don't have unnecessary repo write access; ensure this top-level change remains compatible with the existing job-specific override (the "npm-publish" job already sets its own "contents: read").
🧹 Nitpick comments (1)
.github/workflows/proto.yml (1)
56-59: Consider adding validation for the extractedWASMD_COMMIT.The
awkcommand on line 57 assumes a specificbuf.lockformat. If the format changes or the repository isn't found,WASMD_COMMITwill be empty and the subsequentbuf generatewill fail cryptically.🛡️ Suggested improvement
- name: Generate TypeScript from proto working-directory: proto run: | WASMD_COMMIT=$(awk '/repository: wasmd/{getline; print $2}' buf.lock) + if [ -z "$WASMD_COMMIT" ]; then + echo "Error: Could not extract wasmd commit from buf.lock" + exit 1 + fi buf generate . --template buf.gen.es.yaml --include-imports buf generate "buf.build/cosmwasm/wasmd:${WASMD_COMMIT}" --template buf.gen.es.yaml --path cosmwasm/wasm/v1 --include-imports🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/proto.yml around lines 56 - 59, The current workflow extracts WASMD_COMMIT using an awk one-liner (WASMD_COMMIT=$(awk '/repository: wasmd/{getline; print $2}' buf.lock)) which can yield an empty value; add a validation step after that extraction to check that WASMD_COMMIT is non-empty and matches the expected commit/semver/hash pattern, and if it is empty or invalid, exit the job with a clear error message (e.g., "WASMD_COMMIT not found or invalid from buf.lock") so the subsequent buf generate calls are not run with an empty variable; ensure this validation appears immediately after the WASMD_COMMIT assignment in the workflow run block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/proto.yml:
- Around line 73-81: The publish steps "Publish (latest)" and "Publish (canary)"
will fail without the NODE_AUTH_TOKEN; update both steps to set the environment
variable NODE_AUTH_TOKEN to the repo secret (e.g., NODE_AUTH_TOKEN: ${{
secrets.NPM_TOKEN }}) so npm can authenticate, and ensure the repository has an
NPM_TOKEN secret containing a publish-capable npm token; reference the workflow
steps named "Publish (latest)" and "Publish (canary)" when making the change.
---
Duplicate comments:
In @.github/workflows/proto.yml:
- Around line 17-19: Update the workflow permissions mapping to follow least
privilege: change the top-level permissions entry "contents: write" to
"contents: read" so jobs like buf-build don't have unnecessary repo write
access; ensure this top-level change remains compatible with the existing
job-specific override (the "npm-publish" job already sets its own "contents:
read").
---
Nitpick comments:
In @.github/workflows/proto.yml:
- Around line 56-59: The current workflow extracts WASMD_COMMIT using an awk
one-liner (WASMD_COMMIT=$(awk '/repository: wasmd/{getline; print $2}'
buf.lock)) which can yield an empty value; add a validation step after that
extraction to check that WASMD_COMMIT is non-empty and matches the expected
commit/semver/hash pattern, and if it is empty or invalid, exit the job with a
clear error message (e.g., "WASMD_COMMIT not found or invalid from buf.lock") so
the subsequent buf generate calls are not run with an empty variable; ensure
this validation appears immediately after the WASMD_COMMIT assignment in the
workflow run block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d201ace-4529-4cc2-9ef8-7c2e8ee72b8d
📒 Files selected for processing (2)
.github/workflows/proto.ymlproto/buf.md
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/buf.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@proto/README.md`:
- Line 7: Update the documentation text in the README so the compound adjective
is hyphenated: change the phrase "User facing documentation goes in each
protobuf package following the" to "User-facing documentation goes in each
protobuf package following the" (edit the line containing "User facing
documentation" in proto/README.md); also scan other README or doc files for the
same unhyphenated phrase and apply the same "User-facing" correction for
consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8145716d-17aa-43a4-9bfe-6643305c97a6
📒 Files selected for processing (3)
.github/workflows/proto.ymlproto/README.mdproto/buf.md
💤 Files with no reviewable changes (1)
- proto/buf.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/proto.yml
Description
to publish proto definitions on BSR and npmjs.com
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit