Add a typecheck target, gate it in check, clear its finding

[derek-miller]

install-deps already installed lua-language-server but nothing ran it, so the
LuaCATS annotations were only checked by whoever had it wired into an editor.
Adds a typecheck target against a committed config, puts it in check so CI
enforces it, and installs the server in the Check job.

Note this is unrelated to the existing check-types, which verifies that
src/protobuf/types.lua still matches what the schema generator produces. Both
stay in check.

Two config settings are load-bearing. runtime.version unset makes the server
default to Lua 5.4 and check this library as the wrong language, reporting a
different set of findings rather than fewer. And vendor/ is both a library
and an ignoreDir, so vendored definitions resolve without vendored code being
reported here.

The one finding: @cast field.wireType integer applied to a table field, which
the server only honours on locals, so the cast was silently doing nothing. Hoisted
out of the loop into a local and cast there. No behaviour change.

Server pinned to 3.19.0 in CI because findings move between versions. Verified
clean under 3.18.2-dev and 3.19.0. 174/174 tests pass, luacheck clean.

[svc-finitelabs]

Reviewed at 14bd384, after the ignoreDir follow-up. Everything below was run locally against lua-language-server 3.19.0, the version CI pins. CI is green on this head across all 8 jobs.

The change itself checks out

I re-derived each claim in the description rather than taking it on faith, one knob at a time against the committed config:

config	problems
as committed	0
runtime.version removed	2 deprecated, "Defined in Lua 5.1/5.2/5.5/LuaJIT, current is Lua 5.4"
library: ["vendor"] removed	18 undefined-doc-name, Int64HighLow
vendor dropped from ignoreDir	7, all inside vendor/bitn.lua
both removed	7

Both settings are load-bearing, and load-bearing in the two different directions you describe: the runtime pin changes which findings you get rather than how many, and the library/ignoreDir pair is what separates vendor's definitions from vendor's diagnostics.

The finding is real and the fix is right. Against main's src/protobuf/init.lua, the committed config reports exactly one problem:

init.lua:464  unknown-cast-variable: Unknown type conversion variable `field.wireType`.

The server is not quietly ignoring the cast, it is rejecting the target outright, which is a stronger version of your "silently doing nothing". Zero after the hoist. The hoist is behavior neutral: nothing in the loop body reassigns field, and the dispatch chain below still reads field.wireType directly, so the only read that moved is the one feeding key.

I also confirmed the gate has teeth rather than passing trivially. A planted undefined-field in src/protobuf/types.lua is reported; the identical probe planted in vendor/bitn.lua is not.

The CI step is correct: the 3.19.0 linux-x64 tarball carries bin/lua-language-server at the archive root, so extracting to $RUNNER_TEMP and putting $RUNNER_TEMP/bin on GITHUB_PATH resolves.

Locally: 174/174 tests under luajit, format-check, check-types and typecheck all clean. I could not run lint; luacheck 1.2.0 fails to load under this box's brew Lua 5.5, and it fails identically on main, so that is my toolchain and not your branch.

One thing to fix: the override is only partial

--configpath does not fully displace a developer's local .luarc.json. workspace.* and runtime.* are overridden as documented, but diagnostics.disable is merged in and still suppresses findings. Isolated against the known pre-fix finding, one key per run, everything else held identical:

local .luarc.json	problems
absent	1
{}	1
{"runtime":{"version":"Lua 5.1"}}	1, overridden as claimed
{"workspace":{"ignoreDir":["src"]}}	1, overridden as claimed
{"diagnostics":{"disable":["unknown-cast-variable"]}}	0

Not specific to that one code: with a planted undefined-field probe, disable: [] gives 2 and disable: ["undefined-field"] gives 1.

So the CLAUDE.md sentence "runs lua-language-server against the committed .luarc-typecheck.json, not whatever .luarc.json a developer has locally", and the matching Makefile comment, are true for scope and language version but not for diagnostics. The practical failure is a local false green: make typecheck passes on a dev box and the same commit fails the Check job. Worth either softening the wording to say what is actually pinned, or adding .luarc.json to .gitignore so it cannot be committed either. Nothing in the repo's .gitignore covers it today. It happens to be ignored on my machine by a global rule, which is exactly the kind of thing that does not protect CI.

Stale doc lines

Both places that enumerate what check runs still omit typecheck:

• README.md:165, make check # Run format check, lint, and check-types, and the block under it never gains a make typecheck line even though make help does
• CLAUDE.md:38, # Run all quality checks (format, lint, check-types), which the PR leaves untouched while adding the new section below it
• smaller, same family: README.md:146 describes install-deps as "(stylua, luacheck, amalg, protoc, Python venv)", and it also installs lua-language-server, which now gates check

Optional, your call

useGitIgnore: false plus whole-repo scope means ignoreDir has to enumerate whatever the CI toolchain drops in the workspace, which is what cost the run on a483af9 and what 14bd384 patched with .install, .lua, .luarocks. Scoping the check to src/ removes that coupling:

lua-language-server --check "$(CURDIR)/src" ...

{
  "runtime": { "version": "LuaJIT" },
  "workspace": { "useGitIgnore": false, "library": ["../vendor"] }
}

Tested: 0 problems on this head, still 1 problem on the pre-fix init.lua, and still 0 with a junk .lua/share/junk.lua planted at the root to simulate a toolchain dir, with no ignoreDir at all. The relative path has to be ../vendor, since library paths resolve against the workspace root; a bare "vendor" under src scope silently loses the definitions and hands back the 18 Int64HighLow findings.

The tradeoff is genuine: today's denylist picks up any new top-level Lua automatically, an src allowlist would not. If you expect Lua only under src/, the allowlist is the more stable of the two.

Minor

install-deps does brew install lua-language-server unpinned while CI pins 3.19.0. They agree today, my brew copy is 3.19.0, but given the PR's own point that findings move between versions, that is the drift the pin exists to prevent, still open on the local side.

Not approving, only because of the CLAUDE.md overclaim, which is a doc future readers will trust. Nothing here blocks the merge mechanically.

[svc-finitelabs]

Round 2 at ac92535, re-scoped from 14bd384 (the commit_id my last review actually landed on). Same base c728105, same parent a483af9, different trees: the diff is CLAUDE.md plus one Makefile comment block, no code. Everything below was run on a clean tree with lua-language-server 3.19.0, the version CI pins. Check is green on this head; Build Combined Module was still running when I looked.

The vendor rewrite is right

One ablation per half, against the committed config, whole-repo scope, --checklevel=Warning:

config	problems
as committed	0
library removed (ignoreDir only)	18, undefined-doc-name Int64HighLow
vendor dropped from ignoreDir (library only)	7, all inside vendor/bitn.lua

"with only ignoreDir the vendored definitions are lost and their uses become undefined-doc-name, and with only library the vendored code is diagnosed here" matches both halves exactly. Clearer than what it replaced, and it names the failure mode rather than just the count.

The runtime.version sentence is wrong at the checklevel this target ships with

CLAUDE.md:66:

Unset, the server assumes Lua 5.4 and checks the wrong language. That makes no difference to the findings in this repo today, so it is pinned as the correct setting rather than to change a count.

It makes a 0 to 2 difference, and both are gate-failing:

src/protobuf/init.lua:33:20  [Warning] Deprecated.(Defined in Lua 5.1/Lua 5.2/Lua 5.5/LuaJIT, current is Lua 5.4.) (deprecated)
src/protobuf/init.lua:41:20  [Warning] Deprecated.(Defined in Lua 5.1/Lua 5.2/Lua 5.5/LuaJIT, current is Lua 5.4.) (deprecated)

Those are math.frexp and math.ldexp, the polyfill reads added by #10. Under a Lua 5.4 assumption, reading those names at all is deprecated, so the pin is what keeps this repo's own cross-version compat shims from tripping the check.

I think I can see where the "no difference" measurement came from, because it depends entirely on the checklevel:

invocation, runtime.version removed	problems
--checklevel=Error	0
--checklevel=Warning, what the Makefile passes	2
no --checklevel flag	2

True at Error, false as shipped. And it is the version assumption doing it rather than the absence of the key: an explicit "version": "Lua 5.4" reproduces the same 2. For completeness, "Lua 5.1" gives a different 2 (err-esc on the \xC3\xA9 literal at init.lua:1039), which is the "different set rather than fewer" shape.

The awkward part is that the sentence this replaced was accurate on the count. Your reason for the pin is the better reason and worth keeping in front, it just is not the case that the count is indifferent to it. Something like:

runtime.version is pinned to LuaJIT because that is what Control4 runs. Unset, the server assumes Lua 5.4 and reports the math.frexp/math.ldexp polyfills as deprecated, so the pin is load-bearing for the current check as well as correct on its own terms.

The Makefile comment carries the same claim in shorter form ("not to change a count") and needs the same edit.

Still open from round 1

Re-measured at this head, not carried over.

--configpath only partly displaces a local .luarc.json. CLAUDE.md:55-57 still says make typecheck "runs lua-language-server against the committed .luarc-typecheck.json, not whatever .luarc.json a developer has locally". workspace.* and runtime.* are displaced as claimed; diagnostics.disable is merged and still suppresses. Against a planted two-finding baseline, one key per run, everything else held identical:

local .luarc.json	problems
absent	2
{}	2
{"runtime":{"version":"Lua 5.1"}}	2
{"workspace":{"ignoreDir":["src"]}}	2
{"diagnostics":{"disable":["undefined-field"]}}	1

The practical failure is a local false green: make typecheck passes on a dev box and the same commit fails Check. Either soften the wording to what is actually pinned, or add .luarc.json to .gitignore so it cannot be committed either. Nothing in the repo's .gitignore covers it today.

Both enumerations of what check runs still omit typecheck: CLAUDE.md:38 and README.md:165. Same family, README.md:146 still describes install-deps as "(stylua, luacheck, amalg, protoc, Python venv)" and it also installs lua-language-server, which now gates check.

install-deps brew-installs the server unpinned (Makefile:82) while CI pins 3.19.0. Given this PR's own point that findings move between versions, that is the drift the pin exists to prevent, still open on the local side.

The src/-scoped alternative from round 1 remains an option with a real tradeoff, not a request.

What I did not re-run

Only 3.19.0 is on this box, so "3.18.2 and 3.19.0 agree here" is still your measurement rather than mine. No code changed since 14bd384, so I did not re-run the 174 tests or re-derive the field.wireType hoist; both were clean last round and the trees are identical outside CLAUDE.md and that one Makefile comment. Through the shipped mechanism at this head, make typecheck reports 0 problems.

Commenting rather than approving again, for the same reason as round 1 plus one new one: the override sentence, the stale check enumerations, and now a runtime.version claim a future reader could reasonably take as license to drop the pin. None of it blocks the merge mechanically.

[derek-miller]

You are right and I was wrong in the opposite direction from round 1. Fixed in a1a2b74.

The pin is load-bearing here

Measured at the --checklevel=Warning the Makefile passes, current head, varying only that key:

lua-protobuf         pinned=0   unset=2
lua-noiseprotocol    pinned=0   unset=0
lua-bthome-ble       pinned=0   unset=0

init.lua:33 and :41, the math.frexp and math.ldexp polyfill reads, exactly as you diagnosed. Under a 5.4 assumption those names are deprecated, so the pin is what stops this repo's own cross-version shims failing the gate.

The mistake is worth naming because it is the same one twice. Round 1 I wrote the justification for lua-crypto, where an unset value genuinely caused an 11 versus 18 split, and pasted it into three repos without re-testing. You caught that in noiseprotocol, I verified it there, and then I applied the correction to all three without re-testing either. It is per-repo, and the wording now says so explicitly and only claims it where it was measured.

--configpath merges diagnostics.disable

Reproduced your table. The docs no longer claim the committed config displaces a local .luarc.json; they say what is actually displaced and name the failure mode, a local green that fails Check. Not adding .luarc.json to .gitignore: it is already covered by a global ignore here, and a repo rule would not stop a local file from being merged anyway, which is the part that bites.

Stale enumerations

CLAUDE.md:38 and README.md both now list typecheck under check, and install-deps names lua-language-server.

The unpinned local server

Documented rather than solved. install-deps takes whatever Homebrew has while CI pins 3.19.0, and the target already prints the version, so the docs now tell you to compare it when a local result disagrees with CI. Pinning a brew formula to an exact version is awkward enough that I would rather leave the drift visible than build a fragile mechanism around it. Say if you would rather it downloaded the pinned tarball the way CI does.

Not addressed

The src/-scoped alternative. It is the right long-term answer to ignoreDir drift and it costs tests/ coverage, so it wants a deliberate decision rather than a drive-by change in this PR. Derek has it in front of him.

Nothing here touched code, so the field.wireType hoist and 174/174 stand from round 1. make typecheck reports 0 through the shipped mechanism at this head.

[svc-finitelabs]

Round 3 at e1f855d. The diff from the round-2 head ac92535 is CLAUDE.md and README.md only, zero code, same parent a483af9. Every finding from rounds 1 and 2 is closed. I re-measured the claims the new text makes rather than carrying the old numbers over.

The runtime.version claim is now correct, and stronger than the one it replaced

Round 2's "makes no difference to the findings in this repo today" was false at the checklevel the target actually passes. The new text says unset reports the math.frexp and math.ldexp polyfill reads as deprecated and fails the gate. Measured on a clean tree at this head, with the committed .luarc-typecheck.json and only runtime.version removed, invoked exactly as Makefile:212 does:

config	--checklevel=Warning	exit
as committed	no problems found	0
runtime.version unset	2 problems, deprecated at src/protobuf/init.lua:33 and :41	1

Exit 1, so "fails the gate" is literally true and not just "reports more findings". Naming init.lua's own cross-version shims as the reason is the part that makes the pin defensible rather than incidental.

The --configpath paragraph matches the behaviour

Re-ablated at this head with --configpath pointed at the committed .luarc-typecheck.json, a planted return-type-mismatch probe under src/ as the baseline and a hostile local .luarc.json:

local .luarc.json	findings
none	1	baseline
workspace.ignoreDir: ["src"]	1	displaced
runtime.version: "Lua 5.4"	1	displaced, no deprecated findings added
diagnostics.disable: ["return-type-mismatch"]	0	merged

So the failure mode the paragraph documents, a local disable producing a green run that fails Check, is exactly what the server does. Probe and planted config were removed in the same command and the tree confirmed clean before and after.

The doc lists now match the Makefile

check: format-check lint check-types typecheck at Makefile:220. CLAUDE.md:38 and README.md:165 list all four, README.md:170 adds the typecheck row, and README.md:146 names lua-language-server in the install-deps list.

The unpinned-brew versus pinned-CI gap I raised in round 1 is now documented at the point a developer would hit it instead of being papered over: Makefile:82 is brew install lua-language-server unpinned, .github/workflows/build.yml:57 is LUALS: 3.19.0, and Makefile:211 prints the version so the comparison the doc asks for is actually possible.

CI is green at this head, 8/8: Check, all six Lua versions, and Build Combined Module.

One nit, not blocking

CLAUDE.md:77 says "3.18.2 and 3.19.0 agree here" while the PR body says 3.18.2-dev. A dev build is not the release. I verified neither of those, only 3.19.0, which is what CI pins and what I have locally.

.luarc.json is still absent from .gitignore. I raised that in round 1 and I am dropping it: ignoring the file would hide it from git status, and the note you added above is the thing that actually helps someone chasing a local-versus-CI disagreement.

Approving.