docs: document the typecheck gate and correct CLAUDE.md - #7
Conversation
CLAUDE.md never mentioned typecheck. The target landed in 821ae5c and was gated into check by 32c9c3a, both after the file was last touched, so an agent reading it and running make all skipped the gate CI enforces. - Add the typecheck section, converged with the sibling repos, and note that it scopes to the whole repo because @alias resolves workspace-wide, which is why src/crypto/annotations.lua is never required and still matters. - Document make check as the gate; make all runs neither format-check nor typecheck. make help is stale on this and says so now. - random and openssl_wrapper have selftest() but no benchmark(); crypto.selftest guards on the shape rather than assuming it. - The DriverWorks feature table is regression-tested by the openssl-matrix CI job against real 0.8.5/0.9.2/0.11.1 rocks. openssl_wrapper.selftest() drives injected stand-ins, so it pins the gate's verdict, not the binding's behaviour. - bignum.is_accelerated has a fifth reason string; matching only the four documented ones misses it. - release.yml publishes both crypto.lua and crypto-portable.lua. - ed25519.sign also derives the public key on every call, so the 2.04x needs the public key cached as well as the expanded form. Holding only the expanded form still pays the scalar multiplication. - Add the missing tree entries, correct the CI section, and record that stylua covers src/ only with no .stylua.toml. Trims the design-defence passages; the measured hardware numbers stay.
derek-miller
left a comment
There was a problem hiding this comment.
Approving. This is the most technically load-bearing of the six CLAUDE.md PRs, because the typecheck section is not describing the repo — it is describing lua-language-server's internals to justify how the gate is hardened. Claims like that are the easiest to get subtly wrong and the hardest for a reader to check, so I checked them.
The lua-language-server claims hold, against the pinned version
Fetched script/cli/check_worker.lua at tag 3.19.0, the version build.yml pins:
15: require 'plugin'
163: local function downgrade_checks_to_opened(uri)
166: if status == 'Any' or status == 'Any!' then
167: diagStatus[d] = 'Opened!'
242: -- Downgrade file opened status to Opened for everything to avoid
244: downgrade_checks_to_opened(rootUri)
Both claims are exactly right:
- "force-overwrites only codes whose default status is
Any, leaving everything defaulting toOpenedunder local control" — the guard is literallystatus == 'Any' or status == 'Any!'. So the conclusion follows: anundefined-globalprobe measures a different code class than areturn-type-mismatchprobe, and "probe with a type-check code" is the correct operational advice rather than a preference. require 'plugin'at line 15 confirms theruntime.pluginblind spot is real, not theoretical. A worker that loads plugins before analysis genuinely can be handed anOnSetTextthat blanks every file and still exit clean.
And the framing around the grep is the most valuable thing here. "Treat that as a floor, not a ceiling: its file scope is the shape of its blind spot" is the sentence that stops this section from being dangerous. A grep over script/core/diagnostics/*.lua enumerates diagnostic keys and by construction cannot surface a key consumed during file loading — so runtime.plugin is invisible to the very method offered for finding keys. Documenting the method and the class of thing it cannot find is what makes it safe to hand to someone else.
The enableScheme note is the same discipline at smaller scale: {} being inert for neededFileStatus/groupFileStatus but [] silencing everything for enableScheme is precisely the kind of asymmetry that a type-based intuition gets wrong. Confirmed it is declared as ["file"] at .luarc-typecheck.json:38.
In-repo claims verified
src/crypto/annotations.luaexists with zerorequires of it. That is what makes "workspace-wide; not require'd" worth stating — it looks like dead code and is not.- typecheck runs
--check "$(CURDIR)", the whole repo, so "a narrower scope gives different findings, not fewer" is right and is the reason the previous point matters. check: format-check lint typecheckandall: format lint test build.make helpreally is stale, verbatim:@echo " make check - Run format-check and lint"sits directly under a target that also runs typecheck. Worth fixing in the Makefile at some point, but documenting it is the right call for now, sincehelpis what someone reads first and it is currently lying.tools/generate_srp_vectors.py,run_benchmarks_matrix.sh, and the CI jobs (check,test,openssl-matrix) all exist as described.
Nothing outstanding.
|
Approved with nothing outstanding — please merge. The lua-language-server claims were the ones I most expected to drift, so I checked them against tag 3.19.0 itself: Separately, and not blocking this: |
Part of AGENT-44 (audit the CLAUDE.md files that exist). One PR per repo; this is
the repo you flagged as the drifted one, and it was.
The gap you asked about
CLAUDE.mdnever mentionedtypecheck. The target landed in821ae5cand wasgated into
checkin32c9c3a, both after the file was last touched, so thefile documented
test/bench/format/lint/buildand stopped. An agentreading it and running
make allgetsformat lint test build, which runsneither
format-checknortypecheck, and concludes the repo has no typecheckgate. It does, and CI enforces it.
Added the
typechecksection converged with the four sibling repos, plus the onething specific to this repo: the check deliberately scopes to the whole tree
rather than
src/, because@aliasresolves workspace-wide. That is whysrc/crypto/annotations.luais neverrequired by anything and still matters.Other claims that did not survive checking
"Each primitive module also exposes its own
selftest()andbenchmark()."randomandopenssl_wrapperhaveselftest()and nobenchmark(). Both arelisted as primitives and both are in
crypto.selftest()'s module list, whichguards with
type(mod.selftest) == "function"rather than assuming the shape."pinned as regression cases in
openssl_wrapper.selftest()." That functiondrives injected stand-in bindings, by its own docstring, so it pins the
gate's verdict for a version string rather than the binding's behaviour. Two
rows are weaker than the sentence implies:
KDFis not pinned at 0.8.5 at all(the stub is
"0.9.2"), andrandom(0)raising exists only as prose. The realregression site is the
openssl-matrixCI job, which installs the actual0.8.5-1,0.9.2-2and0.11.1-1rocks and asserts the feature map againsteach. That job was already in
build.ymlwhen CLAUDE.md was last written andthe file never mentioned it.
release.ymlpublishes both artifacts, not justcrypto.lua. It has since963e91a, before the file's last edit. (build.yml's artifact upload keepsonly
crypto.lua, which is the likely source of the confusion.)A fifth
is_accelerated()reason string exists —"OpenSSL modular exponentiation is unavailable", the fallback whenunavailable_reasonreturnsnil. A caller matching the four documented strings misses it. The other four
are verbatim correct.
powmodis what the Control4 build spells it, but the wrapper acceptsbn.powmodorbn.mod_expand pins both.CI section: runs on
mainandmaster, invokesmake test-all, and thecheckjob includes typecheck.The one where the numbers were right and the conclusion was not
The measured tables are excellent and I kept all of them; every figure is dated,
attributed to a binding and OS build, and internally consistent (I re-checked the
arithmetic:
2(0.459)+0.786+1.583 = 3.287,1.602/0.786 = 2.04, the mod_exp fitat 256 and 3072 bits, and
82.05/0.016 ≈ 5100all hold).But "Expand the long-term key once" does not follow from them. Cold sign is
1.602 s, pre-expanded is 0.786 s, and
expand_private_keyis under 0.001 s — sothe 0.816 s gap is not the expansion.
ed25519.signalso derives the public keyon every call (
pt_scalarbase+pt_pack) before delegating tosign_expanded(expanded, public_key, message). A caller that holds only theexpanded form still pays the scalar multiplication and sees none of the 2.04x.
Corrected to say cache both.
Trimmed
Cut the design-defence passages: "This has its own heading because…", "the wrong
precondition twice over…", "Two design points worth not re-litigating", and the
math.randomargument. Kept the measurement inside each (the 5,100x, the ~20bits). Both of the "not re-litigating" points are already comments in
openssl_wrapper.lua, so a reader meets them on opening the file.Verification
make typecheckwith the CI-pinned lua-language-server 3.19.0: clean, noproblems found. Docs-only change, so this is a regression check rather than
evidence for the diff.
checked against the tree.
make test,make build, and theopenssl-matrixjob. No Luasource changed.
Out of scope, worth knowing
src/crypto/utils/init.lua:2reads "Common utility functions for the NoiseProtocol Framework" — copy-paste from lua-noiseprotocol. Not touched here.