chore: pull protos from BSR module instead of vendoring - #11
Conversation
Point buf generate at buf.build/photon-hq/whatsapp and remove the vendored proto/ directory and buf.yaml. Codegen (ts-proto/nice-grpc) and runtime are unchanged; raw protos are no longer shipped in the npm tarball.
📝 WalkthroughWalkthroughBuf generation now references a pinned remote WhatsApp module, CI verifies generated output and project checks, npm packaging excludes ChangesWhatsApp generation and validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…, build Also apply pending Biome formatting to mapper.ts so lint passes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@buf.gen.yaml`:
- Around line 2-3: Pin the buf.build/photon-hq/whatsapp module in the inputs
configuration to an explicit stable commit or tag, ensuring buf generate uses a
reproducible BSR input instead of resolving the latest module.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb5d3b4c-e929-44c0-a144-96ff3ab026b9
⛔ Files ignored due to path filters (1)
src/generated/google/protobuf/timestamp.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
README.mdbuf.gen.yamlbuf.yamlpackage.jsonproto/photon/whatsapp/v1/common.protoproto/photon/whatsapp/v1/media_service.protoproto/photon/whatsapp/v1/message_service.proto
💤 Files with no reviewable changes (4)
- buf.yaml
- proto/photon/whatsapp/v1/media_service.proto
- proto/photon/whatsapp/v1/common.proto
- proto/photon/whatsapp/v1/message_service.proto
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
README.md (1)
67-67: LGTM!package.json (1)
14-14: LGTM!
Makes buf generate deterministic across dev, CI, and the publish-time build; schema updates now require an explicit, reviewable ref bump.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Bun version used by CI.
Using
latestallows an unrelated Bun release to change installation, generation, or build behavior. Use the repository’s pinned toolchain version or a committed version file instead.🤖 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 @.github/workflows/ci.yaml around lines 17 - 19, Update the Bun setup configuration using oven-sh/setup-bun@v2 to reference the repository’s pinned Bun toolchain version or committed version file instead of latest. Preserve the existing CI setup while ensuring future Bun releases cannot change workflow behavior unexpectedly.
🤖 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/ci.yaml:
- Around line 27-32: Update the “Check generated code is in sync” workflow step
to include untracked files when comparing src/generated/ against the repository
state. Ensure newly generated files cause the check to fail, while preserving
the existing error message and generation command guidance.
- Line 15: Update the actions/checkout@v4 step in the CI workflow to set
persist-credentials to false, ensuring the checkout action does not retain
GITHUB_TOKEN credentials for subsequent PR-controlled steps.
---
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 17-19: Update the Bun setup configuration using
oven-sh/setup-bun@v2 to reference the repository’s pinned Bun toolchain version
or committed version file instead of latest. Preserve the existing CI setup
while ensuring future Bun releases cannot change workflow behavior unexpectedly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8d98189-5c64-4073-860c-10082cf732d6
📒 Files selected for processing (2)
.github/workflows/ci.yamlsrc/transport/mapper.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Cursor Bugbot
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
src/transport/mapper.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Files:
src/transport/mapper.ts
🪛 zizmor (1.26.1)
.github/workflows/ci.yaml
[warning] 15-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🔇 Additional comments (2)
.github/workflows/ci.yaml (1)
1-13: LGTM!Also applies to: 21-25, 34-44
src/transport/mapper.ts (1)
513-519: 🩺 Stability & AvailabilityNo change needed for
productItems.OrderContent.productItemsis always initialized to an array, soproto.order?.productItems.map(...) ?? []is safe here.> Likely an incorrect or invalid review comment.
Ryan Zhu (underthestars-zhy)
left a comment
Ryan Zhu (underthestars-zhy)
left a comment
There was a problem hiding this comment.
Andy (@invisicat) does this need release label?
|
Ryan Zhu (@underthestars-zhy) no not yet, i am making changes to proto (adding typing indicator support) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4acb471. Configure here.
| inputs: | ||
| # Pinned BSR commit (main @ spectrum-whatsapp-business 17e7e9c). | ||
| # To pick up schema changes: update the ref, run `bun run generate`, commit both. | ||
| - module: buf.build/photon-hq/whatsapp:b218d3336d65421c8b215137797db344 |
There was a problem hiding this comment.
Stale generated files evade sync check
Medium Severity
buf generate now pulls a remote BSR module, and CI treats a clean git diff on src/generated/ as proof of sync, but buf.gen.yaml does not set clean: true. If a later pin drops or renames protos, old generated files stay on disk untouched, so the sync check still passes and stale types can ship.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4acb471. Configure here.
|
so this is not ready to merge? |
switching proto sourcing from vendored in-tree files to the Buf Schema Registry.
buf generatenow pulls directly from buf.build/photon-hq/whatsapp (tracking main), and the local proto/ directory and buf.yaml are removed.Verification
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Build and release packaging change how schema is sourced and verified; runtime API should stay equivalent if the pinned BSR ref matches the old protos, but CI now gates on BSR availability and committed generated output.
Overview
Proto sourcing moves from in-repo
proto/to a pinned Buf Schema Registry module (buf.build/photon-hq/whatsappinbuf.gen.yaml). Localbuf.yamland all vendored.protofiles are removed;bun run generatenow drives codegen from BSR. Published npm tarballs no longer includeproto—onlydist.CI adds a GitHub Actions workflow on PRs and
mainpushes: install with frozen lockfile, run generate, fail ifsrc/generated/drifts from BSR, then type-check, lint, test, and build.Docs clarify that
generatetargets the BSR module. Regeneratedtimestamp.tshas comment/range churn only; mapper gets a formatting-only tweak on inbound orderproductItemsmapping.Reviewed by Cursor Bugbot for commit 4acb471. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Breaking Changes
Build & Distribution
Documentation
Bug Fixes
productItems(or an empty list).CI