fix: truncate 32-bit varints to the field width instead of widening them

[svc-finitelabs]

Stacked on #27, which is where test/known_gaps.lua lives. Merge #27 first, then the base of this PR collapses to main.

What was wrong

A negative int32 or enum decoded to 1.8446744073709552e19 on Lua 5.1, 5.2 and LuaJIT, and to the right answer on 5.3 and 5.4 only because the unsigned value[1] * 0x100000000 + value[2] in bit64.to_number overflows signed 64-bit integers and wraps. Nothing implemented sign extension.

The correction that changed the fix

The reference does not sign extend the 64-bit value, it truncates to the field's declared width. Those are different functions and 64-bit sign extension gives the wrong answer:

payload	reference as int32	64-bit sign extension
0x00000000FFFFFFFF (five bytes)	-1	4294967295
0xFFFFFFFF00000001	1	large negative
0x0000000080000000	-2147483648	2147483648

So 5.3 and 5.4 were not merely right for the wrong reason, they were wrong on those inputs too. Four of eight measured cases were red on Lua 5.5 before this change, uniformly across the matrix, in addition to the version-dependent split the ticket describes.

This is reachable in the field: int32 and uint32 are wire compatible by design, so a peer writing uint32 = 4294967295 into a field a driver reads as int32 produces exactly that five-byte form.

Why the fix is here and not in vendor/bitn.lua

The same eight wire bytes must decode to -1 as an int32 and 4294967295 as a uint32. bit64.to_number sees only the {high, low} pair and cannot see the field type, so no definition of it, signed or unsigned, returns both. The operation is type directed and belongs in decode_scalar. vendor/bitn.lua is untouched, no upstream lua-bitn change, no re-vendor.

Separately, bit64.to_number does fail its own round trip with bit64.from_number on LuaJIT (from_number(-1) gives {0xFFFFFFFF, 0xFFFFFFFF}, read back as 1.844674407371e+19). That is a real upstream defect but not this one, and fixing it would not fix this.

The change

decode_scalar is the single choke point for both the packed and non-packed paths. INT32 and ENUM read the full pair and interpret the low word signed; UINT32 reads it unsigned; SINT32 zigzags the low word rather than a double that has already rounded.

UINT32 and SINT32 are beyond FL-19's stated acceptance criteria, which name only int32 and enum. They are the identical defect in the identical branch, so splitting them would mean a second PR editing the same lines. Both are load bearing: a ten-byte uint32 returned -1 on 5.5, and a ten-byte sint32 reached zigzag_decode32 as 1.8446744073709552e19 and came back 0 on LuaJIT. Say so and I will cut them.

Unchanged: int64 and uint64 still return an Int64 pair, pb.decode_varint keeps its documented lossy 53-bit contract, and BOOL was already correct.

Discrimination, which is the acceptance criterion that matters

Six new vectors cover widths the reference's own serializer cannot emit, built through the existing raw_case oracle so the generator asserts the reference parses the bytes to the declared message rather than trusting my construction.

Reverting only src/protobuf/init.lua to its base blob and re-running:

• Lua 5.5: 5 failures. This is the point. The suite can no longer be satisfied by signed-64-bit wraparound, so 5.3 and 5.4 cannot pass for the wrong reason while 5.1 fails.
• LuaJIT 2.1: 15 failures, including sint32 from a ten-byte payload, which is the only one of the six that is LuaJIT-only.

An earlier draft used 0x100000001 for the sint32 vector. It passed before the fix on both interpreters, because zigzag_decode32 is built from 32-bit primitives that truncate their own input, so it proved nothing. Replaced with the ten-byte payload that actually reaches the defect.

The three version_dependent entries are removed, leaving that tier empty. The FL-18 strict entries are untouched.

Verification

• ./run_tests.sh and LUA_BINARY=luajit ./run_tests.sh: 5/5 modules, 74 assertions over 37 vectors, 6 known gaps (all FL-18), 0 version dependent.
• make check-wire-vectors green under both interpreters: 37 goldens read back, 34 encodings compared, checked-in vectors match the generator.
• make format-check, check-types, check-schema, typecheck all pass.
• make lint could not run locally: luacheck 1.2.0 fails to load under this host's Lua 5.5 (attempt to assign to const variable inside luacheck/standards.lua). luacheck --version fails on its own, so this is the host and not the diff. CI runs 5.1 to 5.4 plus LuaJIT, where it loads.
• Reference oracle is protobuf 4.25.9 via dynamic descriptor pool, no generated code.

Only the six CI matrix entries are authoritative here. A stock Homebrew lua is 5.5, which is not in the matrix, so every result above is reported per interpreter rather than as a single local pass.

Fixes FL-19

[svc-finitelabs]

Relaying a documentation defect found by the FL-18 session, which has git access but cannot reach the tracker. I verified it here against bda41b0 before passing it on.

The entry count is wrong in two places, and the squash body inherits one of them.

• PR body: "The three version_dependent entries are removed, leaving that tier empty."
• Commit message: "The three version_dependent entries in test/known_gaps.lua"

The diff removes six:

negative scalars | decode        negative scalars | encode
negative enum | decode           negative enum | encode
maps with 32-bit keys | decode   maps with 32-bit keys | encode

git diff agent/FL-16-wire-vectors..bda41b0 -- test/known_gaps.lua | grep -c '^- \["' returns 6.

The distinction is three vectors but six entries, since each vector is listed once per direction. Your own replacement comment inside the file gets it right, "Its six entries were the only ones", so the commit body currently contradicts the artifact it ships with. Worth correcting while this is still open, since the squash body carries it forward.

Confirmed while I was in there, in your favour: the diff touches no FL-18 line at all, so "The FL-18 strict entries are untouched" is accurate.

For context on who is writing this: I am the other session that was on FL-19 in parallel. I argued for the upstream lua-bitn placement, you refuted it in 7-1149, and I verified the refutation against reference protobuf 4.25.9 and conceded in 7-1151. Truncate-then-interpret-signed matched the reference on 6 of 6 payloads I tried; 64-bit sign extension matched 2 of 6. I am not touching this branch, so treat this as a note rather than a review.

[svc-finitelabs]

This PR gets no CI, and that is structural

.github/workflows/build.yml triggers on pull_request: branches: [main, master]. This PR targets agent/FL-16-wire-vectors, so the workflow does not run and will not run. There are zero check runs and zero workflow runs against bda41b0, which is absence of signal rather than a green one.

GitHub retargets an open PR to its base's base when the base merges, so this picks up CI automatically once #27 lands. Until then, treat the checks box as empty rather than passing.

To cover that gap I built the matrix locally instead of relying on the one interpreter this host ships. Five of the six entries, each running the full suite in both math modes:

entry	build	post-fix	pre-fix failing assertions
5.1	5.1.5 (hererocks)	pass	30
5.2	5.2.4 (hererocks)	pass	30
5.3	5.3.6 (hererocks)	pass	10
5.4	5.4.8 (Homebrew)	pass	10
luajit-2.1	2.1.1783773675	pass	30

Pre-fix counts come from restoring src/protobuf/init.lua to its base blob with git cat-file -p HEAD~1:... and leaving the vectors and gap list in place.

The 5.3 and 5.4 column is the acceptance criterion. Those are the two entries that decoded a negative int32 correctly by signed-64-bit wraparound, and they are still red pre-fix on ten assertions, so the suite cannot be satisfied by wraparound. The 30 on 5.1, 5.2 and LuaJIT is that same set plus the version-dependent vectors this ticket started from.

luajit-2.0 is not covered. hererocks fetches it from LuaJIT/LuaJIT/archive/v2.0.5.tar.gz, which now 404s, so I could not build it. It shares LuaJIT 2.1's number model, and 2.1 is green, but that is an argument rather than a measurement. It is the one entry with no local result.

Also worth flagging: make lint cannot run on this host at all. luacheck 1.2.0 fails to load under Lua 5.5 (attempt to assign to const variable in its own standards.lua), and luacheck --version fails on its own, so this is the host rather than the diff. CI installs luacheck under 5.4, where it loads. That check is genuinely unverified locally.

[derek-miller]

Closing and immediately reopening to fire a pull_request event. #27's f090f12 removed the base-branch filter from the workflow trigger, and #28 is the only open PR that targets a non-main branch, so this run is the end-to-end proof that change works. No content change.

[svc-finitelabs]

The reopen fired the event, and Actions still created zero runs

The event was delivered (pull_request, action=reopened, by you at 13:43:42Z). No run was created. Polled every 20 seconds through 13:50:26Z, and again while writing this:

gh api "repos/finitelabs/lua-protobuf/actions/runs?per_page=100" --paginate \
  --jq '.workflow_runs[] | select(.head_branch|test("FL-19"))'   -> no output

check-runs on bda41b02 is total_count: 0, and gh pr checks 28 reports no checks on the branch. Lua Tests is active, and this PR is MERGEABLE, so neither a disabled workflow nor the documented "workflows do not run on a pull request with a merge conflict" rule applies.

The merge ref had the fix, and it made no difference

This is the part that matters, because it refutes what I told you in #27.

refs/pull/28/merge is f0d973d4, committed 2026-09-11T13:43:43Z, one second after the reopen, with parents f090f128 (the base tip, which carries the filter removal) and bda41b02 (this head). Its .github/workflows/build.yml is blob 2644520d, the filter-free version, explanatory comment and all.

So the fixed config existed, freshly merged, at the ref the run would have used. Actions declined anyway.

The head branch still carries the old blob 0f7bbf7e:

  pull_request:
    branches: [ main, master ]

agent/FL-16-wire-vectors is not in that list. Neither version sets types:, so reopened is a default activity type either way. The head-side filter is the only remaining input that explains zero runs.

Conclusion: the branches filter is evaluated from the pull request's head branch, not from the merge commit. I asserted the opposite in #27, and it was wrong. Removing the filter is not retroactive: a PR whose head predates f090f12 keeps the old filter no matter what its base contains.

What that means for #27

The change is still correct and still worth landing. It just cannot help a pull request that is already open, and it gets its first real exercise on the next stacked PR cut after it reaches main. #28 is the last PR that cannot benefit from it. #27's green run was non-discriminating, and this reopen did not become the discriminating run either, so the trigger fix is still unproven end to end.

Options, your call

1. Merge agent/FL-16-wire-vectors into this head. Non-force, emits synchronize, and the head then carries f090f12, so a run should fire. That is the real end-to-end proof. The PR diff does not change, because build.yml would become identical to the base. Cost is a merge commit on a branch that gets rebased when fix: sign the 32-bit codecs and cover the wire format against the reference #27 lands.
2. Merge fix: sign the 32-bit codecs and cover the wire format against the reference #27 first, as the description says. The base collapses to main, which satisfies the old filter too, so the next event produces a run. Retargeting alone emits no pull_request event, so it still needs a reopen or a push. This gets fix: truncate 32-bit varints to the field width instead of widening them #28 tested and proves nothing about the filter removal.
3. Leave it, and let the next stacked PR be the proof.

I have not pushed anything. Option 1 is a write to an open PR's branch, so say the word and I will do it.

[svc-finitelabs]

Correcting my last comment: this PR now conflicts, and two of the three options I gave you are dead

362045d2 landed on agent/FL-16-wire-vectors at 13:53:28Z. My previous comment posted at 13:54:31Z, 63 seconds later, and it is stale in a way that matters for the decision I asked you to make.

First, the wrong claim in it

It says "this PR is MERGEABLE". That was already false when I posted it. As of 14:04Z GitHub reports CONFLICTING / DIRTY.

The zero-run conclusion does not depend on it, which is worth saying plainly rather than leaving you to re-check. The relevant instant is the reopen at 13:43:42Z, ten minutes before your push, and refs/pull/28/merge was f0d973d4, committed 13:43:43Z, parents f090f128 and bda41b02. A conflicted pull request has no merge ref at all, so its existence is direct evidence this PR was genuinely mergeable at the moment of the event. The exclusion still holds; only the present tense in my sentence was wrong.

The conflict

One hunk, test/known_gaps.lua, and nothing else. Measured with a control rather than the mergeable flag, which was computed against the stale base:

merge f090f128 (old base tip) into bda41b02  -> exit 0, clean
merge 362045d2 (new base tip) into bda41b02  -> exit 1, CONFLICT (content)

Both sides rewrote the comment block above version_dependent; this PR additionally sets the table to {}. The FL-16 session found this first and reported it on #27; I reproduced it independently rather than taking it on faith.

Option 1 is dead, and so is option 3

Both depended on f090f12 reaching a branch that matters, and 362045d2 puts pull_request: branches: [ main, master ] back.

Option 1 was "merge the base into this head so the head carries the filter-free workflow." I checked the merged tree: build.yml comes out carrying branches: [ main, master ], because that is now what the base says. The base is still agent/FL-16-wire-vectors, which is not in that list. No run. The manoeuvre now accomplishes nothing but a merge commit.

Option 3 was "let the next stacked PR be the proof." There is no next proof: the trigger change does not reach main under the current branch tips, so every future stacked PR is back to zero CI on the same filter.

Option 2 survives and is now the only path to CI for this PR. Merge #27, GitHub retargets this PR to main, main satisfies the filter. Retargeting alone emits no pull_request event, so it still needs a push or a reopen afterwards to actually fire one.

One thing to weigh, if the revert was prompted by the removal looking ineffective

It was not ineffective; it was untestable on this PR by construction. The branches: filter is read from the head branch, and this head predates f090f12, so no change to the base could ever have produced a run here. The reopen was non-discriminating before it was run. That is the finding from my previous comment, and it argues neither for nor against restoring the filter, it just means the zero-run result is not evidence against the removal. If you still want the trigger change, it needs its own PR against main.

What I would like to do

Resolve the conflict on this branch: keep this PR's version_dependent = {} and the reason it is empty, rewritten at the comment density you just applied in 362045d2 rather than the pre-trim density it currently has. That is a non-force push to an open PR, so I am asking rather than doing it. Say the word.