ir!: the validation constraints WA Web enforces, not just field shapes (schemaVersion 2.0.0) - #36
Merged
Merged
Conversation
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.
Summary
The IR describes what fields a stanza has. That is enough to read a well-formed stanza and not enough to produce one, or to know why a real client rejected the one you produced. This adds the rules WA Web's own parsers enforce — symmetric by construction, so they serve a parser and an emitter equally.
frommust equal the request'stoassertions[].kind == "reference"+referencePath;referencePathon a field for the optional formtype="admin",matched="true",code=429literalValueonParsedFielderrorArms(+errorEnvelope) andkindon a variant"type": "enum"enumRef/enumKeysonParsedFieldnotifications[].actions, andNOTIF_ACTIONSin the referencenotif.rscontentTypetype+byteLength/byteMin-byteMax/literalValueon a content field<registration>(4 bytes)<type>(1)<id>(3);typeElementValuepinned to05Why
Each of these has cost a consumer real debugging time:
build_iq_errorhardcodedfrom="s.whatsapp.net", so every error answering a request addressed tog.uswas unparseable — the client raisedSmaxParsingFailureinstead of surfacing the error. ~50 error sites, mis-filed for months as "the mock does not implement X".<participant type="200">— a status code where the role goes — and the client rejected the whole response of an operation that had already been applied.404 item-not-found, which matches no branch of that RPC. The vocabulary is per-RPC and there was no way to know it except reading each parser.ephemeralDuration(the create payload's spelling) when the action field isduration— every timer reported as 0. Andnot_ephemeralnormalises intoephemeralwithduration: 0, so aNOT_EPHEMERALbranch is dead code.Answerable from the IR alone
Design notes
The error vocabulary is pairs, not two lists.
errorCodes: [400, 501]besideerrorTexts: ["bad-request", "feature-not-implemented"]admits four combinations where the parser accepts two — an emitter picking one from each list sends400 feature-not-implemented, unparseable. 117 variants were ambiguous that way.errorArmspairs them, and the flat lists were removed rather than kept as a convenience view: a convenient view of an unsound shape is still unsound.A two-level error carries an envelope.
SetResponsePreKeySuccessVnameFailurewraps<error code="207">around an inner<error>whose text the disjunction pins. Pins are partitioned by the node they are read from, so the envelope holds only the outer node's and the inner node's shared range stays on the arms — mixing them would say code 207 must also fall in 400–599.The error side comes from the codes, not the name. WA spells the arms inconsistently, and
SetResponsePreKeySuccessVnameFailurereads as a success while assertingtype="error". Classification takes the parser's root discriminator first, then refines by code range. A variant whose codes settle nothing stays a plainerror(45 of them) rather than being guessed into a side, and an error-ish name on a parser that assertstype="result"is a structured non-happy outcome (alternative, 2 of them) — never an error, because the wire discriminator outranks the name in both directions.Required and optional pins are different constraints. A
literalis a hard discriminator — present and exact or the variant does not match. AnoptionalLiteralguards nothing, so it records no assertion; its value still rides on the field. On aBoolpresence flag the pin is always conditional:required: truedescribes the flag the parser always produces, never the wire attribute it reports on.The action union resolves through the handler's own helpers. The
w:gp2arms delegate to module-local functions and the minifier hoists nearly every accessor into a local, so the reader inlines helpers (bounded), followsvarbindings in statement order, and builds a scope per return path — sibling branches routinely rebind the same minified name to different accessors. A branch-only field is optional; a key bound to different wire reads in two branches is dropped rather than resolved to the first.Missing beats wrong, consistently. Ambiguity is refused rather than guessed — a constant table bound twice, a helper name reused by a nested function, a child mapped under one name with two wire tags. Anything seen but not structurally resolvable is counted under
dropsByReason, keyed per parser site, so "no constraint here" and "a constraint we failed to extract" never look alike.Guards
crates/wa-scan/tests/iq_roundtrip.rs) — for every IQ success shape, build a stanza from the IR alone and check the recorded constraints accept it. 140 shapes, 118 constrained, 180 nested pins exercised, against a group-addressed request so a hardcodeds.whatsapp.netfails. It proves the constraints are mutually satisfiable and correctly placed — not that none is missing, since emitter and checker read the same list; the module doc says so.type="admin", both blocklistmatchedpins, and a floor on thefrom→toecho. An aggregate stays healthy while one variant quietly loses one pin; a named assertion cannot.diagnostics.iq.constraintsanddiagnostics.notif.{actions,actionShapes}are regression-checked; the counters may not shrink without--allow-shrink. It fired four times during review: three legitimate reductions (de-duplicated counters, phantomaccount_syncarms) and once on a real regression of mine, before it reachedgenerated/.dropsByReasonnow exists forincomingandnotifas well asiq. A constraint seen and not recovered is counted in the domain whose artifact it belongs to, so the number describes the file beside it:notifreports the one action enum table (create.reason) it cannot name,incomingreports 0.Migration from 1.x
AssertionKindgainedreference— handle or ignore it. (A closed-enum consumer rejects the document: validating the currentiq/index.jsonagainst the 1.0 schema fails 579 times.)ResponseVariantKindgainedclient_error/server_error— a variant that waserrormay now be either. Match all three, or useis_error().errorCodes/errorTexts/errorCodeMin/errorCodeMax/errorClassare gone →errorArms(+errorEnvelope).ContentTypegainedinteger— a<registration>whose body is a number used to be reported asstring.Also non-breaking but worth knowing: enum accessors now decode to
type: "enum"rather than"string"(486 fields already did; 28 disagreed), andmaybeAttrXderives its type fromattrX, which fixed a PN user JID reported as a barestring.Review
Thirty-one rounds, 173 findings from Codex (153) and CodeRabbit (20), all applied or answered with evidence; 170 threads resolved plus 3 raised in review bodies, one declined on the record (see below).
Several corrected wrong values already committed, and a majority of the later rounds were regressions introduced by earlier fixes in the same review — the recurring shape being a predicate that enumerated accessor spellings instead of deriving them from
wap. That bug was found at five separate sites; the fifth is why the "byte-identical codegen" line below now reads differently.One finding was declined on the record — constraint enforcement in the generated parsers; see the follow-ups below.
Follow-ups, deliberately not in this PR
Each of these came up during the work or the review, was measured rather than guessed, and is separable. None blocks this change.
1. Enforce the constraints in the reference codegen. This PR makes the constraints expressible; nothing consumes them yet. The generated parser decodes and validates nothing at all: 2178
literalValue, 169intMin, 88byteLength, 108byteMin, 281enumRefall go unchecked, andassertionsare used only to dispatch a union, never to reject. Codex asked for byte-content validation specifically; enforcing only the newest constraint kind would leave a consumer rejecting an out-of-range byte payload while still acceptingcode=999on a field pinned to 429. The design question is the surface — avalidate()pass, aResultfrom the parser, typed errors — and it should be answered once for every kind.2. Compile the generated Rust in CI.
generated/**/*.rsis gitignored and CI only checks that it parses (syn::parse_file). Building it against a realwacoreis what catches type errors, and it has: a smoke crate found 133 of them in an earlier round. Two of this PR's own bugs are the argument: thecontentUintfields reached generated code, parsed fine as Rust, and decoded binary as decimal text; and an optional JID would have emitted aJidinitializer for anOption<Jid>field — syntactically valid, type-incorrect, and latent only because no such field exists yet. A tiny throwaway crate in CI would close both.3.
previousDocIds— the mex doc-id history. WA rotates a mex operation'sdocIdand the current artifact can only ever show the newest. Measured across the 32 committed versions ofgenerated/mex/index.json: 139 operations, 15 have rotated, 20 retired doc ids are recoverable from a one-shot backfill of git history. It cannot live inmex/index.json— that file must stay a pure function of the pinned bundles for the determinism gate — so it needs a separate accumulator outside the reproducibility chain.wasm.lock.jsonis the precedent. Shape:operationName → { docId → {firstSeen, lastSeen} }.4. Pin the bootloader maps. The extracted structured data (
rsrcMap/compMap/bxData/consistency.rev) is what the discovery path actually depends on, and none of it is pinned today. Pin the extraction, not the raw HTML — nonces and timestamps make the page byte-unstable, so hashing it would fail the determinism gate for reasons that have nothing to do with the protocol.5. A real dataflow pass for the action reader, or a documented scope boundary.
actions.rshas grown a hand-rolled JS scope/control-flow analysis, and the last six rounds of review have been a stream of constructs it does not model (do…whilebreaks,finallywrites,forinitializers, conditional-expression writes, object spreads, by-name callbacks, accessor aliases). Three are still open, recorded on their threads.generated/notif/index.jsonhas not moved across those six rounds, and patching construct-by-construct has itself introduced regressions in five of them — so the class wants a real pass, or an explicit statement of what the reader does and does not model, rather than another round.6. The constraints that are counted but still lost.
dropsByReasonnow names them rather than hiding them, which is the point, but they are real gaps: 37 IQ enum tables WA composes at runtime (babelHelpers.extends({}, base, {error: "error"})), and 1w:gp2action table (create.reason) that is a module-local object rather than a nameable export. Both need a small evaluator rather than a structural matcher.Validation
cargo fmt --all --check;clippy --workspace --all-targetsand--all-features --all-targets, both-D warningscargo test --workspace— 571 tests, 119 added here;cargo test -p wa-ir --features schemacargo check -p wa-ir --target wasm32-unknown-unknown;cargo build -p whatspec --no-default-featuresscripts/validate-schemas.py generated→ 12/12determinism:
update --bundles <pinned> --check→27 committed artifact(s) up to datethe reference Rust codegen gains 26 lines over
main(23 071 → 23 097), verified by regenerating from both and diffingFor most of this review that line read "byte-identical", and that was a bug reporting itself as good news:
wa-codegenstill enumerated three content spellings, so thecontentUintfields the scanners recovered were carried in the IR and dropped at generation. The unchanged output was the symptom. The added lines are those fields —registration: u64,r#type: u64,id: u64and their reads — now reaching generated code.Two rounds later a third defect surfaced in the same file: 108 JID fields the IR marks
required: falsewere declaredJidand parsed withok_or_else("missing …"), because optionality was derived from the accessor spelling rather than from the data. The generated parser rejected responses the IR says may omit them.And one round after the fields arrived they were still decoded wrong:
contentUint(N)is N big-endian bytes, not decimal text (contentIntis the text one), so the first version of those reads rancontent_str().parse()over binary and yielded0every time. They decode fromcontent_bytes()now, andbyteLengthrecords the width.Summary by CodeRabbit