Skip to content

Promote dev -> master: #447 dead-code arc + #310 poll() event loop + maintenance cluster#461

Merged
Aybook merged 18 commits into
masterfrom
dev
Jul 17, 2026
Merged

Promote dev -> master: #447 dead-code arc + #310 poll() event loop + maintenance cluster#461
Aybook merged 18 commits into
masterfrom
dev

Conversation

@Aybook

@Aybook Aybook commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Batch promotion of the validated dev line to master. Merge commit, not squash (§8). Testhub-validated on the :dev image: poll() loop stable over a 2h run (CPU ~0, RAM ~35 MB) under real concurrency + hard disconnect; all touched commands render; German i18n confirmed live; +ban ... -5 rejected; level up/down persists.

Delta (18 commits)

#310 select -> poll event loop (#436) - POSIX now uses poll(), lifting the ~1024-socket FD_SETSIZE ceiling (real ceiling now ~524288, ulimit-bound). Windows stays on select behind the same global_poll name. Boot log names the backend. This is the highest-risk change in the batch and the focus of the testhub run.

#447 dead-code audit arc (8 PRs, all behaviour-neutral):

Maintenance cluster:

Not tagged

No v3.2.0 tag with this merge (maintainer decision). release.yml fires only on v*, so nothing auto-builds. Tagging is a separate deliberate step.

Closes on merge

Part of #447 (tracker closed manually post-merge - the full arc landed). #310 closes with #436's landing on the default branch.

Follow-ups already filed, out of this delta: #444, #453, #456, #457, #459, #460.

Aybook and others added 18 commits July 15, 2026 18:36
…ling (#310) (#436)

The hub's single event loop watches every connected socket in one
socket.select call; luasocket's select.c builds a fixed 1024-bit fd_set,
so at ~1024 sockets the call raised an uncaught error and crashed the
whole hub, dropping every user - the Linux sibling of the Windows #416
crash at 64.

luasocket/src/select.c gains a global_poll backend (variable-length
struct pollfd[] via lua_newuserdata, no FD_SETSIZE cap) registered under
the same `select` name behind a #if defined(_WIN32) guard: POSIX gets
poll, Windows keeps fd_set/select (FD_SETSIZE=1024, #416), the hub side
is untouched. Contract parity with select is exact - check_dirty
force-include of buffered-decrypted TLS sockets, empty-not-nil result
tables on timeout, a socket reportable in both read+write lists in one
tick, errored fds surfaced into every list they were watched in.

hub/hub.c raises RLIMIT_NOFILE soft->hard at boot so the default
ulimit -n = 1024 does not become the new silent cap; getfdlimit() exposes
the resulting ceiling and the boot log reports the backend + ceiling. The
#416 smoke guard became backend-aware (the Linux leg asserts the poll
line, failing pre-fix; Windows keeps FD_SETSIZE >= 1024).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Aybook <265313202+Aybook@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removed duplicate 'bool' function and adjusted indentation.

Co-authored-by: Aybook <265313202+Aybook@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Aybook <265313202+Aybook@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three external PRs (#437, #438, #439) all arrived against master - and
correctly so from the outside: nothing documented the GitFlow-A branch
policy, master is the repository's default branch so GitHub offers it,
and README described 3.2.x as "active development (master)" without
mentioning dev anywhere.

CONTRIBUTING.md leads with the branch table (dev = target this, master =
release substrate, release/3.1.x = security backports only), records that
a mistargeted PR just gets retargeted by a maintainer with the
contributor's authorship intact, routes vulnerability reports to
docs/SECURITY.md, and links out for build / tests / the core
restricted-env use "X" contract / the plugin sandbox / the companion
scripts repo rather than duplicating docs/DEVELOPMENT.md.

README gains a dev-branch pointer directly under the release-lines table
(the sentence that pointed contributors at the wrong branch) plus
CONTRIBUTING.md and the previously-unlisted docs/DEVELOPMENT.md in its
documentation list; DEVELOPMENT.md's orientation table gains a
"your first PR" row.

Also records the #437 / #438 / #439 cleanups in the changelog.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…CPU/RAM (#432)

+hubinfo showed <UNKNOWN> for OS/CPU/RAM on Server 2008 R2 / Windows 7
because core/sysinfo.lua queried via Get-CimInstance (PowerShell 3.0+),
which PowerShell 2.0 (the default there) does not have. Each Windows
probe now runs `try { Get-CimInstance ... } catch { Get-WmiObject ... }`
in one PowerShell call - Get-WmiObject exists in every Windows PowerShell
(2.0-5.1; powershell.exe, not PS-7/Core pwsh), so PS 2.0 hosts take the
catch branch and report real values. Validated live on Windows 11 (+
Get-WmiObject directly). A win_cim(class, property) helper keeps the 4
probes DRY; its args are hardcoded Win32_* literals (no injection).

- core/sysinfo.lua: new win_cim helper + 4 call sites.
- tests/unit/sysinfo_test.lua (new, 21 checks): win/unix parsing + a
  regression that the WMI fallback stays in every Windows command
  (provably fails if removed, §1a.7). Both smoke.yml legs.

Sopor-reported: a hubowner on 3.1.14 additionally CRASHED here
(cmd_hubinfo.lua nil-concat) because the pre-refactor 3.1.x plugin had
no `or msg_unknown` guard; that gets a v0.30 drop-in per §8. 3.2.x.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CMakeLists.txt does find_package(ZLIB REQUIRED) unconditionally for the
Phase 8 S4b zlib_stream (ZLIF) module, but release.yml installed it on
none of its three legs:

  - linux:   build-essential cmake libssl-dev
  - aarch64: build-essential perl wget ca-certificates patchelf file git
             (debian:bullseye-slim, --no-install-recommends)
  - msys2:   gcc cmake make openssl zip

Every other build path already had it - smoke.yml on both legs,
docker/Dockerfile, and the prerequisites documented in docs/BUILDING.md.
Only the release pipeline drifted.

The trap is that libz.so.1 (runtime) is present on virtually every Linux
box because much of userspace links it, so zlib looks installed - but
zlib.h and the libz.so link ship only in zlib1g-dev, which
build-essential does not depend on. Verified from the package database:
`apt-cache depends build-essential` lists no zlib, and a box with gcc,
cmake and OpenSSL headers still had no /usr/include/zlib.h.

Never caught because release.yml only fires on v* tags and release/**
pushes, no v3.2.x tag exists yet, and release/3.1.x predates zlib_stream
- so the release pipeline has never run against a tree containing
find_package(ZLIB REQUIRED). The first v3.2.0 tag would have failed the
aarch64 leg at configure.

Also corrects the CMakeLists.txt comment that legitimised the gap
("aarch64 Bullseye container has zlib as a base-system package - no
change needed") and lists every install site to keep in sync.

Found by a dead-code audit sweep of the build glue.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…442)

etc_cmdlog read lang.failmsg1 / lang.failmsg2, but its lang files define
those messages as msg_denied / msg_nofile. Both lookups returned nil, the
`or "<english literal>"` fallback fired every time, and a German hub
printed English on +cmdlog show regardless of cfg.language - silently, no
error ever raised. Pre-existing since the SourceForge -> GitHub migration.
etc_cmdlog v1.3 -> v1.4.

This is the second instance of the class (#301 PR-2 fixed usr_share's
lang.msg_minmax vs msg_sharelimits), and it surfaced in a plugin the
one-off usr_share_lang_test.lua could not see. Per CLAUDE.md §1a.1 the
guard is now repo-wide: tests/unit/plugin_lang_test.lua enumerates the
shipped plugins from examples/cfg/cfg.tbl's scripts whitelist and asserts
every lang.X reference resolves in BOTH .lang.en and .lang.de (68 plugins,
1034 distinct references). It supersedes usr_share_lang_test.lua, which is
deleted - usr_share is one of the 68 swept, so coverage is a strict
superset and the msg_sharelimits regression stays guarded. Registered on
both CI legs in its place.

The sweep found a third instance, also fixed: cmd_delreg declared
`local msg_reason = lang.msg_reason or "No reason."`, a key no cmd_delreg
lang file defines - copy-pasted from cmd_ban, where the key exists and the
local is used. Harmless (the local was never referenced) but dead either
way, so it is removed. cmd_delreg v0.32 -> v0.33.

Two traps recorded in the test header for anyone extending it:

  - Asserting type(value) == "string" is wrong. ucmd_menu* menu
    structures, month_name and cmd_ascii.pics are legitimately tables; an
    early draft asserting string produced 149 false positives of 151 hits.
    Existence is the invariant.
  - Enumerating via io.popen("ls") is wrong. A native Windows Lua routes
    popen through cmd.exe, finds no `ls`, scans zero plugins and passes
    VACUOUSLY. Hence the cfg-whitelist enumeration (pure loadfile) plus
    explicit minimum-count guards.

Provably fails pre-fix on all three findings (§1a.7): 2271/2277 checks,
exit 1; 2275/2275 and exit 0 with the fix.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… ban (#443)

`+ban nick X -5 reason` stored bantime = -300. At X's next login the
expiry check computed an always-negative remaining, treated the entry as
expired and PRUNED it - so X was kicked once and then walked straight
back in, while the operator believed X was banned. The only time
validation was is_integer( time ), and -5 == math.floor( -5 ), so
negatives sailed through.

Meanwhile help_desc and both lang files advertised "negative values means
ban forever" - a feature deliberately removed in v0.15 ("removed the ban
forever crap"), leaving the promise stranded ever since.

Fixed by rejecting < 1 (new msg_badtime, en + de) at the existing
validation site, which sits before target resolution. The stale promise
is gone from help_desc and both lang files. Zero is rejected for the same
reason (a 0-minute ban expires instantly).

This also closes a divergence per §1a.1: the HTTP path has enforced
min = 1 since #82 (request_schema + a duration_minutes < 1 guard). Only
the older ADC path was broken.

Also removes the commented-out ban-forever block: it referenced
msg_forever, which no lang file defines, so it could never have been
revived by uncommenting anyway.

Note the originally-suspected fix - dropping `[-]?` from the parser - is
a no-op: `%S+` already matches `-5` (verified in Lua 5.4). Validation,
not parsing, was the gap.

A real permanent ban is deliberately NOT reintroduced here; it is tracked
as a separate feature (ADC STA 231 + TL-1, an explicit keyword rather
than a magic negative).

cmd_ban v0.42 -> v0.43.

§1a.7: a new smoke stage sends `+ban nick <nonexistent> -5|0 spam` and
asserts the rejection, with a positive control that a valid time still
reaches the target lookup. Fails pre-fix - the hub answers "User not
found.", proving the negative was accepted and the command ran on to
target resolution.

Found by the dead-code audit.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rence (#446)

* fix(cmd_ban): reject a bantime below 1 instead of silently losing the ban

`+ban nick X -5 reason` stored bantime = -300. At X's next login the
expiry check computed an always-negative remaining, treated the entry as
expired and PRUNED it - so X was kicked once and then walked straight
back in, while the operator believed X was banned. The only time
validation was is_integer( time ), and -5 == math.floor( -5 ), so
negatives sailed through.

Meanwhile help_desc and both lang files advertised "negative values means
ban forever" - a feature deliberately removed in v0.15 ("removed the ban
forever crap"), leaving the promise stranded ever since.

Fixed by rejecting < 1 (new msg_badtime, en + de) at the existing
validation site, which sits before target resolution. The stale promise
is gone from help_desc and both lang files. Zero is rejected for the same
reason (a 0-minute ban expires instantly).

This also closes a divergence per §1a.1: the HTTP path has enforced
min = 1 since #82 (request_schema + a duration_minutes < 1 guard). Only
the older ADC path was broken.

Also removes the commented-out ban-forever block: it referenced
msg_forever, which no lang file defines, so it could never have been
revived by uncommenting anyway.

Note the originally-suspected fix - dropping `[-]?` from the parser - is
a no-op: `%S+` already matches `-5` (verified in Lua 5.4). Validation,
not parsing, was the gap.

A real permanent ban is deliberately NOT reintroduced here; it is tracked
as a separate feature (ADC STA 231 + TL-1, an explicit keyword rather
than a magic negative).

cmd_ban v0.42 -> v0.43.

§1a.7: a new smoke stage sends `+ban nick <nonexistent> -5|0 spam` and
asserts the rejection, with a positive control that a valid time still
reaches the target lookup. Fails pre-fix - the hub answers "User not
found.", proving the negative was accepted and the command ran on to
target resolution.

Found by the dead-code audit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs: correct hci, the event-loop backend, and the logsafe cross-reference

Doc-currency pass from the dead-code audit. Three factual errors:

1. CLAUDE.md §3 listed `hci` in the Infra MODULE map as "(stub)", with the
   row's responsibility ending in "dormant". core/hci.lua is neither: it is
   a persisted DATA file (hubruntime / hubruntime_last_check) that
   hub_runtime rewrites on a 60s onTimer and that cmd_uptime, cmd_hubinfo
   and GET /v1/runtime read. Its absence from init.lua's _core array is
   correct - it is data, not a module - so the old label invited exactly
   the wrong "fix". Removed from the module list and replaced with a note
   saying what it actually is.

   Chasing that down surfaced a real bug, filed as #445 and linked from the
   note: core/ is shipped wholesale (install(DIRECTORY core/), and Docker
   bakes core/ into the image rather than mounting it), so every upgrade
   copies the pristine hubruntime = 0 over the operator's accumulated
   value. This is precisely what the §7 "plugin state lives in
   scripts/data/" rule exists to prevent.

2. CLAUDE.md described core/server.lua as the "select() loop" in both the
   boot diagram and the module map, and §5 still said ">1024 needs the
   select->poll port #310". #310/#436 landed on dev: POSIX now uses poll(),
   Windows stays on select(). My own drift from #436 - §1b.11 says the docs
   move in the same PR as the architecture, and they did not.

3. docs/HTTP_API.md §audit cited `core/http.lua:logsafe` as the source of
   the 512-byte body truncation. That function truncates at 80, not 512,
   and has zero callers - the live implementation is
   core/http_router.lua:logsafe_body (509 + "..."). The cross-reference
   pointed at dead code whose own value contradicted the sentence citing it.

Also refreshes §5's "on dev" paragraph, which still named PR #432 as the
only thing in flight, and points at `git log origin/master..origin/dev` as
the source of truth rather than a list that rots.

Docs only, no code change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
19 file-scope locals declared and never referenced anywhere in the repo,
plus the dead code that only existed to fill them. 83 lines, no
behaviour change: the hub builds, boots and passes the full smoke suite.

Most are Phase-6d extraction leftovers whose consumers moved to
hub_dispatch.lua (four adclib_* aliases, util_difftime, and the _verify /
_normal / _protocol / _identify state-table slots). The rest are older
vestiges: tablesize, loadfile, io, table_concat, types_check, _G,
killuser, usercount, isuseronline, _cfg_nick_change.

Riding along: isuseronline()'s body (defined, never called, a strict
superset of the live isnickonline / iscidonline / issidonline trio),
killuser's 39 commented-out lines, and the hub.usercount export -- never
assigned in hub.lua's history, so the constructor's `usercount = nil`
never created the key and any plugin calling it already got "attempt to
call a nil value".

Two removals were all-or-nothing, because deleting only the declaration
turns the assignment below into a bare global write that init.lua's
restricted env rejects at hub load (the #353 class, which luac -p and the
unit tests cannot see -- only booting the hub does): `local _G` + `_G =
_G` (a no-op, since _G resolves to the nil local on both sides), and
`local isuseronline` + its assignment.

The nick_change FEATURE is live and untouched; only hub.lua's stale cache
slot was dead.

Doc currency: measuring the headroom disproved a claim CLAUDE.md and
docs/DEVELOPMENT.md both carried -- that hub.lua's locals are frozen and
"any new top-level local fails the build". `luac -p -l` reports the main
chunk at 187 locals on dev and 168 here, so the file had 13 free slots
before this PR and has 32 after, and the difference of exactly 19
confirms all 19 removed locals counted toward the cap. Both docs now
state the constraint without a number that can rot again and point at
`luac -p -l` as the way to measure it (plain -p prints nothing; it only
speaks up once you are already over). Four in-code comments asserting the
ceiling in the present tense are reworded as historical rationale; a
fifth already used the past tense and is left alone.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
core/doc.lua is dead three times over, and the third one settles it:

1. commented out of init.lua's _core load list (--"doc",)
2. its export() call site commented out (--doc.export( ))
3. its ENTIRE 19-entry doc.add(...) payload sits inside a --[[ ]] block

So even after undoing (1) and (2), export() would write a file
containing nothing but the "DOCUMENTATION" heading. Zero `use "doc"`
anywhere in the repo, absent from scripts.lua's SANDBOX_GLOBALS and from
docs/PLUGIN_API.md, and its output target docs/documentation.txt has
never existed in the repo's history.

This reverses the 2026-05-23 housekeeping decision to keep the file "for
potential future re-enable" (docs/phases/PHASE_6.md:180, and the
housekeeping entry in the same [Unreleased] CHANGELOG section). That
premise does not survive reading the payload it would re-enable:

- util.savetable/loadtable are documented as exported under the names
  `save` / `load`. They are not - the only occurrences of util.save /
  util.load in the whole repo are doc.lua's own two examples, both of
  which would crash.
- server.wrapsslclient and server.wraptcpclient no longer exist.
- server.add / server.closeall / server.loop are today addserver /
  killall, and there is no server.loop at all (the loop is hub.loop).

Re-enabling would mean rewriting all 19 entries from scratch against an
API they no longer describe. docs/PLUGIN_API.md already covers that
ground and is maintained. This is not dormant documentation; it is a
snapshot of an API that has since been rewritten. Both the CHANGELOG
entry and PHASE_6.md:180 now carry forward pointers, since the reversed
decision and its reversal ship in the same release.

Riding along: init.lua's --"test", and --test( ), dangling since
core/test.lua was deleted in #49.

install(DIRECTORY core/) is wholesale, so no build change. It never
deletes, so an in-place upgrade leaves a stale core/doc.lua behind. That
leftover is inert because nothing calls `use "doc"` - NOT because it is
out of _core: `use` resolves by filename via loadscript and would load a
leftover copy straight off disk if anything asked. Docker is exempt
either way (core/ is baked from source into a fresh runtime image).

Verified by booting the hub with the file genuinely absent from the
install tree: full smoke suite green. Checking that mattered - CMake's
install had left the old copy in place, so a run against an un-cleaned
tree would have proved nothing.

examples/etc/other_available_scripts/cmd_showdoc.lua still calls
doc.collect(). It has been non-functional since the sandbox landed
(`doc` is not a sandbox global, so under the default
no_global_scripting = true the strict env raises "attempt to read
undeclared var: 'doc'"), and it never reaches an install tree
(CMakeLists installs only examples/cfg and examples/certs). Left alone:
examples/ is outside this audit's scope.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
44 lines net. server.lua is the event loop, so this takes only the part
of #447's PR 3 that carries zero judgement: everything here is
core-internal with no caller anywhere.

Two exported functions nothing calls:

- getsettings / changesettings. The nine runtime knobs they read and
  wrote (_selecttimeout, _sleeptime, the buffer caps, the idle timeouts,
  ...) keep their module-level initialisers and every real read, each
  verified individually. They are now the constants they already were in
  practice: changesettings was their only writer and nothing called it.
- stats, whose module-wide _readtraffic / _sendtraffic counters were read
  ONLY there. Dropping them also removes two additions from the read and
  send hot paths.

Plus five dead module locals - table_remove (tick() calls table.remove
directly), socket_bind, signal + signal_set + signal_get, and stop
(declared, never assigned) - and the unused disconnect binding in
wrapclient / wrapserver.

Two easy confusions this avoids: the per-connection readtraffic /
sendtraffic and their accessor handler.getstats are untouched and LIVE
(cmd_userinfo.lua:212); and wrapconnection's disconnect binding is live
while wrapclient's and wrapserver's are dead.

Removing `local signal = use "signal"` drops a `use` call, which can
matter - `use` is `_global[name] or loadscript(name)`. It does not here:
signal is loaded well before server in _core, so the call only ever read
an already-loaded _global entry and never reached loadscript.

DELIBERATELY NOT REMOVED, correcting the audit: the unused
wrapconnection handler accessors id, dispatch, disconnect, bufferlen and
pattern. #447 argued they are unreachable because `server` is absent from
SANDBOX_GLOBALS. That forecloses server.X access but is a non-sequitur
for an object handed to a plugin by reference: user:client() returns
wrapconnection's handler verbatim, and docs/PLUGIN_API.md documents it
both under Network ("Underlying socket handler") and in its type
glossary - where `handler` is the ONLY type not linked to an enumerated
method section, being defined instead as "a wrapped socket from
core/server.lua". The doc delegates the surface to the source. Decisive:
getstats appears NOWHERE in docs/, yet cmd_userinfo depends on it through
that exact route, so non-enumeration cannot mean non-contract.
bufferlen and pattern are setters; a third-party plugin may be tuning
buffers through them.

Note the audit's list spans two different objects: handler.socket and a
second handler.id live on wrapserver's handler, which no plugin route
reaches (user:client():socket() would be a nil call - wrapconnection
never defines .socket). Those are genuinely dead and left to PR 3b on
their own merits, not on the contract argument above.

addclient / wrapclient and the never-called starttls branch remain the
judgement calls in this file and stay open on #447.

Behaviour-neutral: builds, boots, full smoke suite green against a tree
with the patched file installed.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#451)

139 lines. server.lua is now 1148, down from 1331 before PR 3a. These
are the two judgement calls deferred from #450, plus the two accessors
that PR's review reclassified.

1. addclient + wrapclient + socket_tcp

addclient was exported but called by nothing in-tree - and never
plugin-reachable either, since `server` is absent from scripts.lua's
SANDBOX_GLOBALS (http_client is in it). So the export was unreachable
by construction, not merely uncalled. wrapclient and socket_tcp were
reachable only through addclient.

It was also broken. On the `timeout` retry path it discarded
wrapclient's return, so `handler` stayed nil while `err` was still
"timeout" from the connect probe - the very condition the branch tested.
It returned `nil, "timeout", id` for a connect that was in fact
proceeding, since wrapclient had already armed the deferred wrap that
would dispatch it. A caller would have abandoned a live connection as
failed. wrapclient additionally called listeners.failure(...) unguarded.

Outbound TCP is not lost. core/http_client.lua does its own non-blocking
connect over socket.tcp + server.addtimer, and that is the path every
outbound consumer already chose: etc_regserver_announce (the #279
hublist announcer), etc_blocklist_feeds, etc_proxydetect and
etc_status_push all go through http_client. None ever used addclient.

server.lua's own comment (from #188) recorded that addclient was
knowingly bug-fixed while caller-less, because "a divergent broken copy
is a defect". That is a keep-it-correct decision, not a keep-it one.

2. The deferred-TLS handler.starttls branch

Never called from anywhere - and it never worked. The functioning
startssl path installs the handshake coroutine as the event-loop entry
point (handler.readbuffer = handshake). starttls instead wrote
handler.receivedata / handler.dispatchdata: field names the event loop
never reads, assigned from handler.handshake, which does not exist (the
handshake coroutine is a local), so they got nil besides. The real
defect is what it fails to do - it leaves readbuffer / sendbuffer on the
PLAINTEXT handle_read_event / handle_write_event while ssl_wrapping the
socket underneath. A caller would get a TLS socket driven by plaintext
handlers, stuck mid-handshake, because the handshake can never resume
across a readiness event.

ADC has no STARTTLS in any case (adcs:// is TLS from connect). This is
Prosody net.server_select / XMPP heritage no ADC path could ever call.

Dead companions: needtls (written twice, never read), wrapconnection's
`local shutdown` (assigned twice, never called - distinct from
wrapserver's handler.shutdown, which is LIVE via hub.lua's _servers
sweep), and changetimeout (declared, never assigned - the same class PR
3a swept `stop` for and missed).

The else branch's handler.readbuffer / handler.sendbuffer assignments
are KEPT. sslctx-without-startssl is unreachable from hub.lua today, but
addserver takes the two independently, so the branch is structurally
reachable; removing it whole would leave such a connection with no
read/write handlers at all.

3. handler.socket and handler.id on the wrapserver handler

Per #450's review correction. That object reaches no plugin:
add_server_handler is a hub.lua local and addserver's return lives in a
private _servers table whose only use is `s.shutdown()`. user:client()
returns wrapconnection's handler, which has no .socket at all. The
identically-named wrapconnection accessors ARE contract surface and stay.

Behaviour-neutral: builds, boots, full smoke suite green - including the
TLS login path, which a byte-level diff confirms is unchanged.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
33 lines from util.lua, 2 from adc.lua. All no-ops today, but each is a
landmine for whoever touches the function next.

Proven behaviour-neutral mechanically: the old and new util.lua were
loaded side by side under Lua 5.4.8 and both versions' output compared
across every input class. All 35 cases identical. `git diff -w`
collapses the util.lua change to pure deletions plus one edit - the
dedent is forced by removing an enclosing `if`, not a refactor.

1. Dead type guards in encode / decode / trimstring

`local str = tostring( str )` sits above each guard, and tostring never
returns nil - tostring(nil) is the truthy string "nil". So the condition
is always satisfied and the error path is dead. Verified at runtime:
util.encode(nil) returns the encoding of "nil" on old and new alike.

Worse, encode/decode's dead branch read `tbl` - undeclared in that
scope, a fossil of a table-taking ancestor's signature. Had it fired
under init.lua's restricted env it would have raised "attempt to read
undeclared var" instead of returning (nil, err). Disarmed only by being
unreachable.

Behaviour is preserved exactly: all three still stringify whatever they
are given. That is a real hole - util.trimstring(nil) returns the string
"nil", and the companion repo's etc_requests feeds it unvalidated chat
input - but closing it is a breaking change on sandbox-exposed functions
needing tests, a PLUGIN_API update and a coordinated etc_requests fix,
since hardening trimstring would turn that plugin's silent misbehaviour
into a crash. Tracked in #453.

2. Four unreachable type( x ) ~= "number" guards

In formatseconds, formatbytes and difftime (x2). Each sits after
`local x = tonumber( x )` AND an `if not x then return nil, err end`.
Since tonumber returns a number or nil and nothing else, the two guards
fire on IDENTICAL inputs - measured across every input class. The type
check is not a missing validation, it is the redundant second copy of
one; `if not x` is the live guard and stays.

This supersedes #439's merge rationale deliberately. That merge kept the
guards knowing they were unreachable ("Worth having regardless: the
expression is a latent trap the moment that `tonumber` rebind is ever
refactored away. Merging as-is"). The unreachability is not a new fact -
what is corrected is the reasoning: in that scenario `if not x` is the
guard that survives, so the type check was never the safety net it
appeared to be.

Attribution: #439 (@Kcchouette) corrected the expression in
formatseconds and formatbytes only, and corrected it rightly -
`not type( x ) == "number"` really does parse as
`( not type( x ) ) == "number"`. difftime's two guards date to the
SourceForge import and were always written correctly.

UNTOUCHED: convertepochdate and generatepass use the same shadow but
have NO preceding nil-guard, so their type checks are reachable and
load-bearing - generatepass's is precisely what stops `nil < 0` from
raising.

3. adc.lua's duplicate FC / TO keys

In the STA named-parameter table, declared once for base STA and again
under the ASCH block. (#447's text and this PR's first draft said SCH.
Wrong table - SCH is 140 lines further down and has no FC at all.
Caught in review.) Unlike the #438 `bool` duplicate, both writes store
the identical value, so last-wins changes nothing today. The trap is
future-tense: changing the base FC would be silently overridden back by
the ASCH copy, and the edit would look like it did nothing.

Builds, boots, full smoke green - util.lua and adc.lua are on every code
path the harness exercises.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
88 lines: 61 declarations, their 6 orphaned assignments, 2 commented-out
remnants, and 2 section headers left labelling nothing.

Mostly stdlib and module aliases that outlived their consumers. Six were
bind-late `local X` / `X = ...` pairs assigned and never read: out_put in
util and in cfg_users, out_error in cfg_secret, types_adcstr and
_cfgbackup in cfg, _integer in adc. (cfg_secret's out_put is LIVE - five
calls - and stays; only its out_error was dead.)

whitelist's _rebuild_indices was copy-pasted from blocklist, whose only
caller is a bulk_replace whitelist does not have. _integer lost its
consumer when adc's _regex.integer was rewritten inline for upstream
luadch#241.

cfg's _cfgbackup is not an unfinished feature - it is a finished one,
deliberately disabled: cfg.lua once wrote timestamped cfg.tbl.backup
copies on every cfg.set; that call was commented out in 2016 and #272
deleted the remnant, orphaning _cfgbackup AND cfg's os_date. No data is
at risk - cfg.set still persists via util_savetable(_settings,
"settings", _cfgfile); only the backup copies stopped, ten years ago.

Derived by a purpose-built detector rather than the tracker's list:
comments stripped first, every file-scope local counted, iterated to a
fixpoint so cascade orphans surface. It found four the list did not -
adc's `debug`, cfg's `types`, hbri's `cfg` (each orphaned only BY
removing its last consumer) and hub.lua's `tonumber`, missed by PR 1
because that PR worked from the same short list.

It needed two corrections of its own, both from review: `X = X,` in a
return table is an EXPORT, not an assignment; and `local function X`
parsed as a local named `function`, hiding that whole class - which made
the first draft's "0 candidates remain" false until a reviewer named
whitelist's orphan.

LOAD ORDER IS UNCHANGED, the one thing that could have broken. `use "X"`
on an unloaded core module triggers its loadscript. mem is _core
position 2, ahead of util (4), out (7), adc (18); `debug` is a stdlib
global with no core/debug.lua shadow; hbri is not in _core. The one real
case is cfg's `use "types"` (cfg 5, types 28): it resolves because
cfg.lua loads cfg_defaults itself at :529, which does `use "types"` ~50
lines further into the same load. Nothing between touches types, and
types.lua is passive at load.

The six all-or-nothing pairs are proven safe by bytecode scan: zero
GETTABUP/SETTABUP _ENV "<name>" across every patched file. A wrongly
removed live local would necessarily compile to exactly those, so this
is categorical. Hub boots, full smoke green.

Deliberately kept: util.handlebom and cfg_secret.is_active are now
orphaned in-tree but live on plugin-reachable tables (#447's
conservative-on-contract-surface rule). http_events' _max_size is read
only by its own clamp, so http_events_buffer_size may do nothing - a
behaviour question, not dead code.

Part of #447

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#455)

DEVELOPMENT.md documented the `use` load-order rule in one direction only
("a module that does use "X" at load time must appear after X in the
array"). The #447 cleanup arc kept walking into the inverse, which was
written down nowhere:

- Deleting a `local X = use "Y"` is NOT free. `use` is
  `_global[name] or loadscript(name)`, so for a module not yet loaded
  that call IS what pulls Y in. Removing an unused local can therefore
  move a module's load. PR 5 hit the real case: removing cfg's unused
  types_* aliases orphaned its `use "types"`, and cfg sits at _core 5
  while types is 28 - safe only because cfg.lua loads cfg_defaults
  itself, which does `use "types"` ~50 lines further into the same load.

- luac -p proves syntax, not scope. The failure that matters - a deleted
  local whose assignment or read survives - is syntactically perfect and
  dies at hub load under the restricted env. Two checks catch it: a
  bytecode scan (`luac -p -l -l | grep '_ENV "name"'`; GETTABUP = dangling
  read or captured closure, SETTABUP = stray global write, zero hits is
  proof) and booting the hub. Both are worth writing down because the
  unit suite passes on code that cannot load, so "tests green" is not
  evidence here.

Also note to validate the scan against a name known to be global in that
file first - a grep pattern's silent zero fooled this arc three times.

Part of #447

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each removed local is a `local msg_x = lang.msg_x or "<fallback>"` that
its own plugin declares and never reads; their keys go from both .lang.en
and .lang.de. Four keys were already orphaned with no local at all
(cmd_reg ucmd_passwort, etc_userlogininfo msg_ccpm_1/2/3).

None of the 18 hides a swallowed error path - checked before deleting,
not after: cmd_userinfo denies via msg_god at the hierarchy check,
cmd_ban announces via report.send, blocklist/whitelist do report add /
export / import failures (with the helper's raw string - #456),
etc_clientblocker's silence on a missing .tbl is the deliberate
seed-on-missing path from #81 (the source marks it the canonical
first-run path rather than an error), and etc_trafficmanager uses its
regged lookup only to derive target_level.

Derived twice independently (§1a.4). The tracker says 17 locals / 40
lines; both are wrong. A subagent's per-file lexer and a second detector
reusing the shipped guard's comment-stripper each returned the same 18
out of 1021 declarations. 18 decls + 18 .en + 18 .de is not 40 either
way. The likely miss is cmd_accinfo's two ucmd_menu_ct* to an [a-z_]+
pattern that silently skips digits - the class that hid sha256 in PR 5.

The four dead plugin exports #447 lists here are NOT removed. Their
criterion - absent from PLUGIN_API.md §8's table - does not survive
reading §8, which closes with "See each plugin's source for the full
exported surface". It documents 7 of the 18 exporting plugins, omitting
etc_usercommands.add (210 call sites in the companion repo), cmd_ban.del,
cmd_help.reg and etc_hubcommands.has/list. Non-enumeration cannot mean
non-contract - PR 3a's getstats lesson, inverted. Tracked as #457.

plugin_lang_test.lua now asserts both directions. The #442 guard checked
only that every lang.X read resolves; an unread key was invisible to it,
which is how these 22 accumulated. It also under-scanned: `lang%.([%w_]+)`
cannot see `lang. msg_notfound` (whitespace after the dot, which Lua
accepts and usr_uptime.lua:143 is written with), so that key was
unguarded - and a reverse check without %s* would have called it orphaned
and deleted a live German translation, i.e. the #442 bug reintroduced by
the sweep meant to prevent it. Both directions now tolerate it. The
reverse pass has its own MIN_REVERSE_KEYS vacuity floor.

Provably fails pre-fix (§1a.7): 8 failures on the unpatched tree
(cmd_reg ucmd_passwort + etc_userlogininfo msg_ccpm_1/2/3, each in .en
and .de). Patched: 4280/4280 over 68 plugins.

No scriptversion bumps - DEVELOPMENT.md scopes those to semantic changes
(behaviour, cfg keys, wire surface) and is how the companion repo syncs.
This has none, and none of the 12 plugins exists there.

Part of #447

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Aybook
Aybook merged commit 6ea8662 into master Jul 17, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants