build(codegen): own the whatspec codegen and regenerate at 2.3000.1044659339 - #1293
Conversation
…ed IR artifacts whatspec stopped committing Rust: generated/**/*.rs is gitignored there and only the language-neutral IR ships, so refreshing a vendored file is no longer a `cp` and nothing checked that the vendored set stayed coherent. It did not: abprops and appstate described WhatsApp 2.3000.1042742319 while mex and the token tables described 2.3000.1044659339, and the default app version a fresh device announced was a fourth hand-maintained copy of the same number. whatspec-codegen reads the IR at a commit pinned in whatspec.lock.json and emits every derived file in one pass, refusing to run when the domains disagree on a build. Acquisition is git rather than HTTP so it works wherever `git clone` does, sparse to `generated/`, with the per-file sha256 checked on top so a local `--from` checkout is held to the same bar. `--check` reproduces the tree byte for byte and is wired into CI. The emitters reproduce the shapes already committed, proven by regenerating from the older pinned IR and getting the current files back unchanged. Local additions stay declarative: a retained proto message lives in LOCAL_BLOCKS beside the LOCAL_FIELDS hook, and a sync that reclaims the name fails the build instead of shadowing it.
….1044659339 One pass of whatspec-codegen, so the whole set now describes one build instead of two. The version itself is generated as wacore::version::WA_WEB_VERSION and is what a fresh device announces, which removes the last hand-maintained copy of the number. What moved with the bump: - abprops gains the WAWebHybridABPropsConfigs registry (339 flags) as `abprops::hybrid`, and the web registry goes 2036 -> 2193. Every flag this client reads survives, and both `props::stale` entries are still absent upstream, so the module stays correct. - appstate drops DeviceCapabilities, adds LabelSublist, and moves enum-field metadata between MarketingMessage, WasaRootSecret and StatusPrivacy. schemas_unlisted::LABEL_MESSAGE is still absent from the registry, so it stays. - The proto picks up DeviceProps.PlatformType::WAIL, mapped to the OtherWebClient companion type for the same reason the other non-browser platforms are: it needs attestation this crate cannot produce, and '0' is server-rejected. - The proto also renames the AIRichResponseContentItemsMetadata oneof upstream, so waproto's `a_i_rich_response_content_item` becomes `ai_rich_response_content_item`. Breaking for anyone matching that oneof by name; migration is the rename. - tokens.json is unchanged apart from gaining its missing trailing newline.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded the ChangesWhatspec code generation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
| Filename | Overview |
|---|---|
| tools/whatspec-codegen/src/main.rs | Coordinates pinned IR acquisition, coherent artifact generation, write/check modes, and descriptor hash validation; the previously reported check-mode gap is fixed. |
| tools/whatspec-codegen/src/source.rs | Acquires pinned or local IR and verifies the lock revision and per-file digests. |
| tools/whatspec-codegen/src/emit/proto.rs | Generates the vendored protobuf schema while preserving declarative local message additions. |
| tools/whatspec-codegen/tests/committed_artifacts.rs | Adds offline tests enforcing common version stamps and token-table indexing invariants. |
| .github/workflows/main.yml | Adds a bounded CI job that verifies all generated artifacts against the pinned IR. |
| waproto/src/whatsapp.proto | Regenerates the WhatsApp protobuf schema from the newly pinned whatspec bundle. |
| wacore/src/version/generated.rs | Adds generated constants tying the default announced client version to the vendored artifact version. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Pinned whatspec commit] --> B[Verify IR digests and version stamps]
B --> C[Generate Rust, JSON, and proto artifacts]
C --> D{Mode}
D -->|write| E[Write generated files]
E --> F[Run protoc and update descriptor hashes]
D -->|check| G[Compare generated text artifacts]
G --> H[Validate proto and descriptor hashes]
H --> I[Generated Artifacts CI passes]
Reviews (4): Last reviewed commit: "fix(codegen): fail before writing when p..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53641afda8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 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/main.yml:
- Line 46: Update the actions/checkout step in the workflow to set
persist-credentials to false, ensuring the checkout does not store the
GITHUB_TOKEN while preserving the existing action version and job behavior.
- Around line 37-45: Add a timeout-minutes value to the generated job definition
alongside runs-on, limiting the vendored artifact validation workflow so stalled
fetches or whatspec-codegen builds cannot block the required check.
- Around line 46-51: Update the workflow steps around actions/checkout@v6 and
dtolnay/rust-toolchain@master to derive the toolchain from rust-toolchain.toml
after checkout, then pass the parsed channel to the action’s toolchain input
instead of hardcoding nightly-2026-06-16. Preserve the rustfmt component and
subsequent cargo run check.
In `@tools/whatspec-codegen/src/emit/abprops.rs`:
- Around line 46-60: Validate that ir.configs is sorted by module before the
grouping loop in the emitter containing module_idents and render_module. Reject
or assert any later config whose module precedes the previous module, so
interleaved modules fail loudly instead of producing duplicate unique_ident
values; preserve the existing grouping behavior for valid sorted input.
In `@tools/whatspec-codegen/src/emit/appstate.rs`:
- Around line 161-179: Update render_enum to generate variant identifiers
through the crate’s existing unique_ident mechanism, deduplicating pascal_case
collisions while preserving each value’s as_str mapping; remove the unnecessary
Clone bound from its values iterator. Ensure the collection call sites that
format Collection variants reuse the same deduplicated names rather than calling
pascal_case independently.
In `@tools/whatspec-codegen/src/emit/mex.rs`:
- Around line 29-31: Update the builder setup in the operation emission flow to
reserve the exact top-level names Variables and Response in Builder::by_name
before registering op.variables_shape and op.response, ensuring nested objects
receive suffixed names and the generated API always exposes these names.
- Around line 108-119: The emitters must make generated identifiers unique after
normalization. In tools/whatspec-codegen/src/emit/mex.rs:108-119, update
register to maintain a per-struct used_fields set and pass each rust_ident(key)
through unique_ident; apply the same fix in the TypeNode::Object branch at lines
138-142. In tools/whatspec-codegen/src/emit/appstate.rs:161-179, update
render_enum to pass each pascal_case(v) through unique_ident and reuse the
resulting variant map at the Collection:: formatting sites on lines 85 and 99.
- Around line 173-182: Update scalar_rust so the "number" tag maps to a
float-capable Rust type instead of i64, while preserving the existing boolean
mapping and String fallback for other tags.
In `@tools/whatspec-codegen/src/main.rs`:
- Around line 306-324: Update rustfmt to use a unique scratch file for each
invocation instead of the fixed scratch.rs path, ensuring concurrent processes
cannot overwrite one another’s input or formatted output. Generate the unique
filename within target/whatspec-codegen while preserving the existing rustfmt
execution and readback flow.
- Around line 122-153: Update parse_args to reject --from when combined with
--update-lock, alongside the existing --check/update-lock contradiction
validation. Ensure the combination returns an error before any lock update
proceeds, while preserving valid local-source and update-lock usage
independently.
In `@tools/whatspec-codegen/src/naming.rs`:
- Around line 10-16: Add "try" to the RUST_KEYWORDS list in naming.rs so JSON
keys named try use the existing raw-identifier conversion path and emit valid
Rust 2024 code.
In `@tools/whatspec-codegen/src/source.rs`:
- Around line 177-192: Update verify around the actual.get lookup to return a
contextual error when an IR_FILES entry is missing instead of panicking with
expect. Preserve the existing lock comparison and error reporting, and identify
the missing file using the rel symbol.
- Around line 154-174: Update resolve_rev to parse all git ls-remote output
entries and require an exact ref-name match rather than accepting the first line
or a ref-name tail match. Prefer the peeled ^{} SHA for annotated tags, reject
ambiguous or missing exact matches, and validate the selected SHA is exactly 40
hexadecimal characters before returning it.
In `@wacore/appstate/src/schemas.rs`:
- Line 1264: Update the generation source rather than editing generated
schemas.rs or protobuf output: restore device-capability coverage for
DEVICE_CAPABILITIES and DEVICE_CAPABILITIES_V2, or route unsupported actions
through schemas_unlisted.rs as appropriate. Inspect the emitter/IR and
value_enum_fields handling to restore the STATUS_PRIVACY mode and modes mappings
when required for decoding. Regenerate both artifacts and verify they agree with
the declarations in whatsapp.proto.
In `@wacore/src/companion_reg.rs`:
- Around line 114-115: Add P::WAIL to the platform cases exercised by
unconfirmed_platform_types_collapse_to_other, ensuring the test asserts it
collapses to C::OtherWebClient alongside the existing SMARTGLASSES case.
🪄 Autofix
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6f33869b-8add-43f7-b194-0f7651878eba
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.github/workflows/main.ymlAGENTS.mdCargo.tomlagent_docs/wa_web_reference.mdsrc/version.rstools/whatspec-codegen/Cargo.tomltools/whatspec-codegen/src/emit/abprops.rstools/whatspec-codegen/src/emit/appstate.rstools/whatspec-codegen/src/emit/mex.rstools/whatspec-codegen/src/emit/mod.rstools/whatspec-codegen/src/emit/proto.rstools/whatspec-codegen/src/emit/tokens.rstools/whatspec-codegen/src/emit/version.rstools/whatspec-codegen/src/ir.rstools/whatspec-codegen/src/main.rstools/whatspec-codegen/src/naming.rstools/whatspec-codegen/src/source.rstools/whatspec-codegen/tests/committed_artifacts.rstools/whatspec-codegen/whatspec.lock.jsonwacore/appstate/src/schemas.rswacore/binary/src/tokens.jsonwacore/src/companion_reg.rswacore/src/iq/abprops.rswacore/src/store/device.rswacore/src/version.rswacore/src/version/generated.rswaproto/src/whatsapp.descwaproto/src/whatsapp.desc.sha256waproto/src/whatsapp.proto
…t name collisions The Generated Artifacts job inherited the workflow's RUSTC_WRAPPER=sccache without installing sccache, so cargo failed before it ran anything. The emitter changes are guards, not corrections: the committed artifacts still reproduce byte for byte, so none of these collisions occur in the pinned IR. All of them would have produced a generated file that does not compile, or a silent public-API rename, while the generator exited zero. - mex: reserve `Variables` and `Response` before walking either shape. Children are interned before their parent, so a field named `response` would take the bare name and rename the operation's own struct to `Response2`. - mex: route field names through `unique_ident`. `fooBar` and `foo_bar` are one Rust field, and a struct cannot declare it twice. - appstate: deduplicate enum variants and reuse that map at every `Collection::` site, and reject an action naming a collection the IR never declared. A new `unique_type_ident` suffixes without the underscore, since `CriticalBlock_2` would trip `non_camel_case_types` in the generated file. - abprops: reject an IR whose configs are not grouped by module, which would split one registry across `web` and `web_2`. - naming: `try` is a reserved keyword; a JSON key named `try` emitted a field Rust 2024 rejects. - source: resolve `--rev` against an exact ref name. `git ls-remote` matches ref-name tails, so `main` could pin `refs/heads/feature/main`; prefer the peeled sha for annotated tags and require 40 hex digits. - source: `verify` returns an error rather than panicking on a missing IR file. - main: reject `--from` with `--update-lock`, which would pin a remote rev against digests read from an unrelated local tree, and give rustfmt a per-process scratch file so two runs cannot swap each other's output.
|
Pushed The CI failure was mine: Applied, with a test each. Worth stating plainly: these are guards, not corrections.
Not applied, three:
Generated by Claude Code |
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 `@tools/whatspec-codegen/src/emit/appstate.rs`:
- Around line 77-85: Preserve the variant map returned by render_enum for Scope
instead of discarding it with .0, and use that map when emitting each action’s
scope in the appstate generation flow. Replace the independent
pascal_case(&action.scope) formatting with a lookup that fails loudly if the
scope is missing, matching the existing Collection handling. Add a regression
test covering scope names that normalize to the same PascalCase variant and
remain distinct.
In `@tools/whatspec-codegen/src/main.rs`:
- Around line 395-396: Remove the duplicated rationale comment near the test
case in the relevant test function, leaving the test name and assertion to
describe the scenario. Preserve the rationale comment at the lock-validation
site around the existing validation logic.
🪄 Autofix
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6abbd6b3-8150-4980-86dc-bd677c3619a9
📒 Files selected for processing (8)
.github/workflows/main.ymltools/whatspec-codegen/src/emit/abprops.rstools/whatspec-codegen/src/emit/appstate.rstools/whatspec-codegen/src/emit/mex.rstools/whatspec-codegen/src/main.rstools/whatspec-codegen/src/naming.rstools/whatspec-codegen/src/source.rswacore/src/companion_reg.rs
…and check mode Four findings from the Codex and Greptile reviews, all real, all in the part of the tool that is supposed to make drift impossible. - The version-coherence loop only read `index.json`, so `proto/WAProto.proto` was never compared against the manifest. It is the one domain with no JSON envelope, and therefore the one whose drift would reach `whatsapp.proto` with every other registry agreeing. Its `/// WhatsApp Version:` header is now part of the same check. - `--update-lock` wrote the lock before `build()` ran any emitter validation, so a lost proto anchor or an invalid token table left the lock pinned to a build no committed artifact describes. The lock is now the last thing written. - `--check` returned after comparing the six text artifacts, so a tree regenerated with `--skip-proto-desc`, or a hand-edited `.desc`, passed the Generated Artifacts job and only failed later when another job built `waproto`. Check mode now compares both hashes recorded in `whatsapp.desc.sha256`, which needs no protoc. - appstate routed `Collection` through the deduplicated variant map but left `Scope` deriving its name independently. That is the worse half: two scopes normalizing alike compile fine and give one action the other's scope, and a scope escaped by `ensure_ident` (`self` to `Self_`) names a variant that was never declared. The committed artifacts still reproduce byte for byte.
📦 Binary size report
.text per crate
Top movers (cargo-bloat attribution)
🚨 Per-PR size budget exceeded (Δ stripped ≤ 64.00 KiB, Δ .text ≤ 32.00 KiB):
The Baseline: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf18d4d8bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…a partial IR cache Two follow-up findings, both about a run that stops halfway leaving state a retry cannot repair. - `write` replaces `whatsapp.proto` but not the descriptor beside it, so discovering a missing protoc afterwards left the two halves describing different schemas, in a tree that does not build and that looks regenerated. protoc is now checked before anything is written. - `fetch_pinned` reused a cache entry whose `generated/` directory merely existed. An interrupted checkout therefore poisoned the cache permanently: every later run skipped the fetch and failed reading the missing files, with no way out but deleting the directory by hand. Reuse now requires every IR file to be present.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Summary
whatspec stopped committing Rust.
generated/**/*.rsis gitignored there and only the language-neutral IR ships, so "refreshing a vendored file is acp" has been false for a while and nothing checked that the vendored set stayed coherent. It did not:abprops.rsandappstate/schemas.rsdescribed WhatsApp 2.3000.1042742319 whilemex_operations.rsandtokens.jsondescribed 2.3000.1044659339, and the default app version a fresh device announced was a fourth hand-maintained copy of the same number.This adds
tools/whatspec-codegen, a non-published workspace member that reads the IR at a pinned commit and emits every derived file in one pass, and regenerates the whole set at 2.3000.1044659339.Design
Acquisition is git, not HTTP. The lock pins a commit; fetching that commit reuses whatever credentials, proxy and CA configuration the user's git already has, so the tool works wherever
git clonedoes. The checkout is sparse togenerated/(~10 MB, cached undertarget/). Per-file sha256 digests are checked on top, because they are also what makes a local--fromcheckout trustworthy. No new dependencies:anyhow,serde,serde_json,sha2, all already in the tree at the same versions.Regeneration is all-or-nothing. Every domain document has to stamp the same
waVersion— includingproto/WAProto.proto, which carries its stamp as a comment rather than a JSON envelope and is therefore the one domain whose drift could otherwise reachwhatsapp.protowith every registry agreeing. The manifest has to agree, and the IRschemaVersionmajor has to be one this tool reads. A tree assembled from two whatspec runs stops the run rather than producing the drift this PR removes.Reimplemented rather than depended on. Depending on whatspec's
wa-codegenfrom git would put a git source in the workspace lockfile and four more crates in everycargo clippy --workspace, to get output that is explicitly untracked and "reference" quality upstream. The emitters here are ~700 lines and we control the shape.Local additions stay declarative.
waproto/src/whatsapp.protocarries two messages WA dropped from the public bundle (LIDMigrationMapping,LIDMigrationMappingSyncPayload). Those now live inLOCAL_BLOCKSin the proto emitter, spliced after a named anchor, alongside the existingLOCAL_FIELDShook inwaproto/build.rsfor single fields. A sync that reclaims one of those names fails the build instead of shadowing it, and a sync that removes the anchor fails instead of silently dropping the block.Writing the
.protoalso rerunsscripts/regenerate-proto-desc.sh, soprotoc(thePROTOC_VERSIONinmain.yml, 3.25.3) has to be on PATH;--skip-proto-descdefers it.--checknever rebuilds the descriptor — it compares the two hashes recorded inwhatsapp.desc.sha256instead, which is why the CI job needs only rustfmt and git and still catches a tree regenerated with--skip-proto-desc.Correctness of the reimplementation
The emitters have to keep mapping bundle keys to the same public names, or a refresh silently renames API. Proven, not asserted: pinning the tool at the older whatspec commit (2.3000.1042742319) and running
--checkreproduces the currently committedabprops.rs,appstate/schemas.rsandwhatsapp.protobyte for byte, local proto block included. The three that did not match under that pin are exactly the three already known to be from the newer bundle, and they match byte for byte when pinned to it. The descriptor is reproducible too: regeneratingwhatsapp.descfrom the unchanged.protowith protoc 25.3 gives the committed sha256.Changes
tools/whatspec-codegen(new,publish = false, outsidedefault-members): IR model, naming, six emitters, git acquisition with a sha256 lock,--check.wacore/src/iq/abprops.rs,wacore/src/iq/mex_operations.rs(already there, unchanged),wacore/appstate/src/schemas.rs,wacore/binary/src/tokens.json,waproto/src/whatsapp.proto+.desc+.desc.sha256.wacore/src/version/generated.rs(new, generated) carriesWA_WEB_VERSION/WA_WEB_VERSION_STR, re-exported fromwacore::versionandwhatsapp_rust::version.Device::defaultuses it, so the announced version and the vendored registries can no longer disagree.abpropsgains theWAWebHybridABPropsConfigsregistry asabprops::hybrid(339 flags); the web registry goes 2036 to 2193. Every flag this client reads survives, and bothprops::staleentries are still absent upstream.appstatedropsDeviceCapabilities, addsLabelSublist, and moves enum-field metadata betweenMarketingMessage,WasaRootSecretandStatusPrivacy.schemas_unlisted::LABEL_MESSAGEis still absent from the generated registry, so it stays where it is.Generated Artifactsjob running--check.AGENTS.mdandagent_docs/wa_web_reference.mdupdated: thecpinstruction was wrong, and the generated-vs-hand-written boundary is now written down where someone about to edit a generated file will read it.Breaking changes
cargo-semver-checksreports these against the last published release — all of them upstream schema changes carried through by the regeneration, none of them referenced anywhere in this workspace:waproto: upstream renamed theAIRichResponseContentItemsMetadataoneof, soa_i_rich_response_content_itembecomesai_rich_response_content_item. Migration is the rename.waproto: upstream removedEncryptMessageOutput.messageKey(field 6), soEncryptMessageOutput::message_keyand its…Viewcounterpart are gone. Nothing in this crate constructs or reads that message; a downstream consumer that does needs to drop the field.wacore:DeviceProps.PlatformType::WAILis new. It maps toOtherWebClientfor the same reason the other non-browser platforms do: the Android letters need attestation this crate cannot produce, and'0'is server-rejected. A downstream match onPlatformTypeneeds the new arm.The
Semver Checks (informational)job iscontinue-on-error: trueand reports exactly the twowaprotoremovals above (6 findings = 2 fields × struct andView, some listed twice).Guarantees
tools/whatspec-codegen/tests/committed_artifacts.rsis the offline half of the gate: it needs no whatspec checkout, so it runs in the ordinary matrix and catches the failure that actually happened, a single domain refreshed on its own. It also checks the token tables still start with the reserved empty slot, sincetokens.jsoncarries no version stamp and an off-by-one there corrupts every frame.waVersion, a proto anchor that vanished, a proto name upstream reclaimed, a proto stamped from another build, an IR not grouped by module, a collection or scope the enum never declared, and two wire strings that normalize to one Rust identifier.Cost
The proto grew by ~19 KB of descriptor and the abprops registry by ~490 flags. The flags are
pub constand unreferenced ones emit nothing, but the extra proto messages will move the binary-size gate; if the increase reads as expected, it needs thesize-increase-oklabel.Validation
All green locally. CI status is on the checks tab; the only expected failure there is
Semver Checks (informational), for the breaks listed above.