Skip to content

Build and test-tree integrity: guard mk nuke, stale bytecode, and dis paths - #550

Closed
Ticed wants to merge 7 commits into
infernode-os:masterfrom
Ticed:fix/build-and-test-tree-guards
Closed

Build and test-tree integrity: guard mk nuke, stale bytecode, and dis paths#550
Ticed wants to merge 7 commits into
infernode-os:masterfrom
Ticed:fix/build-and-test-tree-guards

Conversation

@Ticed

@Ticed Ticed commented Aug 25, 2026

Copy link
Copy Markdown

What this changes

Six small guards and one document, all about the build and the test tree
telling the truth about their own state. No functional change to the
emulator, the VM, or any Limbo application.

**mk nuke at the repo root no longer deletes the tracked dis/ runtime
tree.** Root nuke walks $DIRS ($EMUDIRS + appl), and appl's nuke
deletes $DISBIN - which for appl is the committed dis/, about 900
.dis files a fresh clone needs in order to boot. Afterwards the emulator
dies with panic: loading "/dis/emuinit.dis": ... does not exist and
git status shows a wall of deletions that reads like a bad merge. The
rule now refuses when dis/emuinit.dis is tracked, and names the two ways
forward: mk emunuke for an emulator-only clean, or NUKE_DIS=1 mk nuke
to really do it. cd appl && mk nuke is deliberately left unguarded.

**build-macos-sdl3.sh restores include/version.h when the build fails.**
It stamps the version before compiling, so a failed build left the stamped
file behind, surfacing as an unrelated modification in the next
git status.

**build-macos-headless.sh bootstraps utils/ndate before the emulator**,
rather than failing partway through on a fresh clone.

**tools/verify-dis-paths.sh now covers the whole appl and tests tree.**
It guarded appl/cmd only, so the wrong-target bug it exists to catch - a
module whose PATH constant names one location while the build installs to
another - could still land anywhere else. Sources with no inline PATH
constant are reported as skipped rather than silently ignored, since mk
TARG governs those.

**run-tests.sh refuses to run test bytecode older than its source.** A
stale .dis carries the assertions it was compiled with, so it fails
against code that is correct, and there is no link typecheck error to
explain why unless a .m interface also changed. The failure names an
assertion, which sends you into the wrong file; the natural next move is to
edit correct code until it matches a stale test. The check lists the
offending binaries and points at mk install.

**docs/WORKFLOW-TRAPS.md** records these traps and two related ones and
is linked from DOCUMENTATION-INDEX.md.

Verification

  • mk nuke at the root refuses with exit 1 and dis/ is intact afterward
  • run-tests.sh refuses with the stale binary named after touching a test
    source, and proceeds once the binary is rebuilt
  • tools/verify-dis-paths.sh: OK: 10 sources have fresh .dis at their declared PATH (1053 skipped: no inline PATH constant, mk TARG governs those)

macOS ARM64. The two behavioural guards were watched failing and then
passing, not merely inspected.

Scope

Deliberately excluded to keep this to one logical change: the
tests/mkfile TARG audit and its pre-commit guard, which need a fix to
the tests they would wire in (which follow separately).

@Ticed
Ticed requested a review from pdfinn as a code owner August 25, 2026 12:43
@pdfinn

pdfinn commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thanks for this — the diagnosis in here is good, and two of the traps you
identified turned out to be worse than the PR describes. Reviewing it sent me
down a long path, and the result is that most of this is now overtaken by
#559 and #560. Detail below so the reasoning is visible rather than just the
verdict.

Please split out and I'll take them

build-macos-headless.sh bootstrapping utils/ndate. Real, and confirmed
by grep: ndate appears in no other .sh or .yml in the tree, so nothing
else covers it. One ask — build-macos-sdl3.sh has the identical hole and is
the build AGENTS.md lists first, so please fix both.

The version.h sed pattern. Tightening to s|InferNode 0.1 (| is a real
improvement: the old pattern double-stamped on a second run, the new one is
idempotent. Worth having on its own.

One change to the mechanism, though: please restore version.h from a saved
copy rather than git checkout --. On an EXIT trap that now fires on every
run, and 2>/dev/null || true hides it, so a developer with an in-progress
version bump loses it silently. cp aside and cp back does the same job
without touching anything they wrote.

Superseded by #559 / #560

Not because the problems weren't real — because they've been fixed further
upstream, at the condition that caused them.

The mk nuke guard. nuke was doing exactly what it means in Inferno:
remove build products, installed ones included. What was anomalous is that we
tracked dis/ while the build system treated it as output. #560 stops tracking
it, and root mk nuke now leaves zero git changes — I ran it to check. The
wall of deletions can't happen, and recovery is a 14-second rebuild rather than
git checkout -- ..

Two notes on the guard as written, for the record. It was Posix-only, so &-Nt
and &-Inferno were unguarded and Windows is a shipped platform. And the
premise is overstated: mkfiles/mkdis's nuke-std is cd $DISBIN; rm -f $TARG
— it removes the TARG-listed names, not rm -rf $DISBIN. Still several hundred
files including emuinit.dis, so the symptom you describe is real; "about 900"
isn't.

The verify-dis-paths.sh rewrite. Removed entirely in #560. Three reasons,
and I'd have raised the first regardless of the rest:

  1. It fails open. Stub the scanner to return non-zero and the script prints
    OK: 0 sources ... and exits 0. Both the pre-commit hook and the CI job gate
    solely on that code, so a broken scan is a silent green.
  2. The file set broadened, the parser didn't. Of the 25 sources under appl/
    and tests/ carrying a /dis/ PATH constant, it recognises 8.
    appl/xenith/render/mdfmt.b:20 and htmlfmt.b:27 declare their own install
    path at file scope rather than inside a Foo: module {} block, so they land
    in the bucket labelled "no inline PATH constant" — which isn't true of them,
    and they're the nested-subdirectory shape the tool exists to guard.
  3. mk already holds the answer. The header argues that parsing mkfiles would
    re-implement mk's DIRS/include logic badly — agreed, which is why the check
    should ask mk rather than re-derive from the .b files with awk. Make the tracked runtime tree exactly what the source compiles to #559 does
    that: it rebuilds and compares. That also dropped the bash dependency, which
    was running against CONTRIBUTING.md:226.

The run-tests.sh staleness guard. The class it catches can no longer occur
— bytecode is never committed, so it can't be stale relative to its source. One
observation while it existed: it skipped sources with no .dis at all
([ -f "$_dis" ] || continue), and this tree has 136 tests/*_test.b against
98 binaries, so 38 test sources produced nothing and the suite went green without
them. That's what your deferred tests/mkfile TARG audit covers; #559 wires the
missing targets in.

The lib/guide speech line

You were right and I was wrong when I first read this. I flagged /n/speech
/mnt/speech as incorrect because every call site uses /n/speech. That's
backwards: docs/NAMESPACE-LAYOUT.md decides placement by who authored the
schema
, speech9p invents its own (Qctl/Qsay/Qhear/Qvoices), so /mnt/speech
is correct — the same case as webfs/mnt/web, and the doc even names this
failure mode for the sibling service ("despite the long habit of writing
/n/llm").

The change is still incomplete rather than wrong: speech9p.b:98 and the call
sites in lucibridge.b and luciconv.b still use /n/speech, so doc-only makes
the guide correct-by-convention and wrong-in-practice on the same day. Worth
doing as a real migration under its own ticket.

What the investigation turned up

Since it started here, the things this PR was circling around:

  • appl/cmd/git/push.b hadn't compiled since 2026-07-02 (fix(security): attenuate each tool invocation namespace #338 removed
    readcredentials() and left it called)
  • dis/acme.dis shipped stale for five months, referencing
    /fonts/vera/… after the source moved to /fonts/combined/…
  • a Veltro tool removed by a commit titled chore(security) was still shipping
    as bytecode in the agent's tool directory
  • developer paths (/home/user/…, four /.claude/worktrees/…) inside released
    .dis files
  • 45 modules shipping that no mkfile ever compiled

Test suite, pristine master vs #559, both built from scratch: 64 passed / 76
failed → 121 passed / 21 failed. 56 test files fixed, zero regressions.

Also filed from this: INFR-428 (the test runner's namespace leaks into the
Inferno sh phase) and INFR-429 (JIT faults on Linux arm64 under -c1) — the
jitbug/jitshift repro cases #559 restores may help with the latter.

Happy to be argued with on any of the above, particularly the mk nuke reading.

@Ticed

Ticed commented Aug 28, 2026

Copy link
Copy Markdown
Author

Thanks — taking the split you asked for.

#565 carries the two you wanted: utils/ndate in both build scripts, and
the version.h stamping. The restore takes a copy before stamping and puts
that back rather than git checkout --, so an in-progress edit survives; a
test covers that and the second-run case.

#564 is the mk/limbo preflight, also in both scripts. It came out of a
separate ticket, and the exit-status half of that ticket did not reproduce on
master — set -e is already there. Worth noting the ndate line in that
report came from this branch, which is why it looked like master behaved
differently.

Dropped, per your reading: the mk nuke guard, the verify-dis-paths.sh
rewrite, and the run-tests.sh staleness guard. No argument on mk nuke — the
anomaly was tracking dis/, and nuke removing build products is what it
means. The fail-open point on verify-dis-paths.sh is the one that should have
stopped me writing it that way.

The lib/guide speech path needs speech9p.b:98 and the lucibridge.b /
luciconv.b call sites moved with it, so it wants its own change rather than a
doc-only edit. Not carried here.

The tests/mkfile TARG audit is in progress separately and stacks on #559/#560.

Closing this in favour of #564 and #565.

@Ticed Ticed closed this Aug 28, 2026
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