Skip to content

Commit eb0cb3c

Browse files
committed
fix: editor-sanitizer CI job actually used clang, not gcc as documented
An external review of tonight's work caught a real discrepancy: plan.md's SYS-W11-09 writeup claims "the new sanitizer job [uses gcc/g++] too, matching" the plain editor job, but `git log -S"editor-sanitizer"` shows the job used clang/clang++ since its very first commit (6e356c3) -- that claim was never actually applied to ci.yml. The job has never had a fully green CI run: first blocked by unrelated environment gaps (fixed separately tonight), now by SYS-W8-08's sharp-runtime API mismatch, unrelated to compiler choice. Fixed for real, not just in prose: switched the editor-sanitizer job's CC/CXX to gcc-14/g++-14 (matching the plain editor job and the local build-asan/ verification, which was already GCC-based, confirmed via its CMakeCache.txt) and swapped clang for g++-14 in its apt-get install list. Downgraded SYS-W11-09 from [DONE] to [IN_PROGRESS] in plan.md: the code fixes (heap-use-after-free, 4 leak fixtures, meshcraft_apply_sanitize() wiring) are real and independently verified locally, but the CI job itself was never actually verified green and had this real misconfiguration. Also corrected NEXT.md's "Current priorities" section, which had gone stale mid-session (still claiming "no Wine in this sandbox" and "awaiting Windows run" hours after both were resolved and documented further down the same file) -- exactly the kind of internal contradiction the same review flagged.
1 parent 5b98f69 commit eb0cb3c

3 files changed

Lines changed: 62 additions & 24 deletions

File tree

.github/workflows/ci.yml

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,17 @@ jobs:
367367
name: Editor (EASYGL) ASan+UBSan
368368
runs-on: ubuntu-24.04
369369
env:
370-
CC: clang
371-
CXX: clang++
370+
# Corrected 2026-07-27: this job used clang/clang++ since its very
371+
# first commit, contradicting this file's own SYS-W11-09 writeup in
372+
# plan.md ("the new sanitizer job [uses gcc/g++] too, matching" the
373+
# plain `editor` job below) -- that claim described intent that was
374+
# never actually applied here. GCC's ASan+UBSan support is equally
375+
# complete; switching removes the untested risk of hitting the
376+
# documented Clang/CNA::Internal::JsonValue incompatibility, and keeps
377+
# this job on the same toolchain as the plain `editor` job for a
378+
# sanitizer finding that can't be blamed on a compiler difference.
379+
CC: gcc-14
380+
CXX: g++-14
372381
ASAN_OPTIONS: detect_leaks=1:halt_on_error=1
373382
UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1
374383
steps:
@@ -414,7 +423,7 @@ jobs:
414423
run: |
415424
sudo apt-get update
416425
sudo apt-get install -y --no-install-recommends \
417-
clang cmake ninja-build python3 pkg-config \
426+
g++-14 cmake ninja-build python3 pkg-config \
418427
libgl1-mesa-dev libegl1-mesa-dev libglvnd-dev \
419428
libx11-dev libxext-dev libxrandr-dev libxcursor-dev \
420429
libxi-dev libxfixes-dev libxss-dev libxtst-dev \

NEXT.md

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,36 @@ distribution rather than missing editor breadth.
3030

3131
## Current priorities
3232

33-
1. Obtain native standalone Windows qualification (`SYS-W11-06`). The
34-
release-readiness changes are published on `develop`; await the next
35-
remote `windows-2022` CTest/artifact evidence run — the first real run
36-
found 6 concrete regressions (see "Session log" below), all 6 now fixed
37-
and pushed, but none re-verified on an actual Windows runner yet (no
38-
Wine in this sandbox).
39-
2. The freshly-added `[PROPOSED]` backlog (`SYS-W9-06/07`, `SYS-W1-08`,
40-
`SYS-W2-06`, `SYS-W11-08/09/10`, `SYS-W13-03`, `SYS-W3-05`) is
41-
**all 9 DONE**. `SYS-W11-09` found a real heap-use-after-free bug in
42-
`EventPreviewRunner::execute()`'s RunScript step (`ff62004`) — a stale
43-
`working.scripts` iterator read again after `LuaScriptRunner::run()`
44-
already replaced `working` via move-assignment. Fixed. See `plan.md` for
45-
full evidence on everything done.
46-
3. All 6 of the deferred CI-red regressions from that first Windows run are
47-
now fixed and pushed (see "Session log" below for each one's root cause
48-
and evidence) — still awaiting real Windows re-verification for the 3
49-
Windows-specific fixes (`mc3_roundtrip`, `mcb_load_policy`, `mc3togltf`
50-
DLL staging).
33+
_(This section is a summary snapshot — it was left stale for several hours
34+
during the 2026-07-27 session while only the "Session log" further down got
35+
updated. Corrected once, after an external review caught the contradiction;
36+
if you're reading this much later, re-check `plan.md`'s own status markers
37+
before trusting anything below.)_
38+
39+
1. **`SYS-W11-06` (standalone Windows qualification) is `[DONE]`**, verified
40+
for real against the actual published `windows-2022` CI artifact
41+
(downloaded via `gh run download`, run through Wine in this sandbox —
42+
Wine is NOT blocked here for console binaries, only the full GUI editor
43+
hits `SIGSYS`). See `plan.md`'s own entry for the full evidence trail,
44+
including a genuine first-attempt failure (`STATUS_DLL_NOT_FOUND`) that
45+
led to statically linking the whole standalone-Windows build.
46+
2. The `[PROPOSED]` backlog from the 2026-07-26 review (`SYS-W9-06/07`,
47+
`SYS-W1-08`, `SYS-W2-06`, `SYS-W11-08/10`, `SYS-W13-03`, `SYS-W3-05`) is
48+
done. **`SYS-W11-09` is `[IN_PROGRESS]`, not done** — its code fixes (a
49+
real heap-use-after-free in `EventPreviewRunner::execute()`, 4 leak
50+
fixtures, `meshcraft_apply_sanitize()` wiring) are real and locally
51+
verified, but the new `editor-sanitizer` CI job itself has never had a
52+
fully green run: it used `clang`/`clang++` despite `plan.md` claiming
53+
otherwise (corrected 2026-07-27, now `gcc-14`/`g++-14`), and currently
54+
fails on `SYS-W8-08`'s unrelated `sharp-runtime` API mismatch.
55+
3. All 6 originally-deferred CI-red regressions, plus 10 more layers
56+
cascading from fixing them, are fixed and confirmed on real CI (see
57+
"Session log" below) — `Clang ASan+UBSan and bounded fuzz` and
58+
`Standalone Windows qualification` are fully green. The 3 Editor CI jobs
59+
now get all the way through CMake configure and fail only at build, on
60+
2 real `sharp-runtime` bugs tracked as `SYS-W8-07`/`SYS-W8-08`
61+
deliberately not fixed (sibling repository, outside authorized scope;
62+
user chose to report and defer).
5163
4. `SYS-W11-11` (the first-release version-number decision) is deliberately
5264
left `[BLOCKED]` per the user's explicit choice not to decide yet.
5365

plan.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ report and defer rather than fix).
375375
linking the standalone Windows build. See `SYS-W11-06` for the full
376376
verification.
377377

378-
- **SYS-W11-09** `[DONE]` `P1` — Added first-party editor sanitizer CI, and it
378+
- **SYS-W11-09** `[IN_PROGRESS]` `P1` — Added first-party editor sanitizer CI, and it
379379
immediately found a real bug, confirming the whole point of doing this.
380380
Root `CMakeLists.txt` already had `-DMESHCRAFT_SANITIZE=ON` wired up, but
381381
`meshcraft_apply_sanitize()` was only ever called on the main `MeshCraft`
@@ -444,13 +444,30 @@ report and defer rather than fix).
444444
these fixes regressed the normal configuration.
445445
New `editor-sanitizer` CI job (`.github/workflows/ci.yml`): checks out the
446446
same pinned CNA/sharp-runtime revisions as the plain `editor` job, builds
447-
with `gcc`/`g++` + `MESHCRAFT_SANITIZE=ON` + EASYGL, runs `ctest -LE
448-
render`, then the one render smoke test with leak detection scoped off.
447+
with `MESHCRAFT_SANITIZE=ON` + EASYGL, runs `ctest -LE render`, then the
448+
one render smoke test with leak detection scoped off.
449449
**Note for a future session:** `build-asan/` (a stable, reusable directory
450450
per the top-level build-rules convention) is ~5.7 GB on disk after this
451451
verification; left in place for incremental reuse rather than deleted,
452452
since another session may want to re-verify against it.
453453

454+
**Correction (2026-07-27, external review caught this):** the paragraph
455+
above originally claimed the new CI job builds with `gcc`/`g++` "matching"
456+
the plain `editor` job — false. The job used `clang`/`clang++` from its
457+
very first commit; only the *local* verification (`build-asan/`, the
458+
181/181 result above) actually used GCC. This is now fixed for real
459+
(`CC: gcc-14`/`CXX: g++-14` in `ci.yml`, not just corrected prose), but
460+
**downgrading this task's status to `[IN_PROGRESS]`**: the `editor-
461+
sanitizer` CI job itself has never had a fully green run on real CI —
462+
every run through 2026-07-27 failed, first on unrelated environment gaps
463+
(fixed separately, see the CI-red session log in `NEXT.md`), now on
464+
`SYS-W8-08`'s `sharp-runtime` API mismatch (unrelated to compiler choice,
465+
will likely still block after the GCC switch). The code fixes described
466+
above (heap-use-after-free, 4 leak fixtures, `package_consumer_smoke`
467+
guard, `meshcraft_apply_sanitize()` wiring) are real and independently
468+
verified locally; only the CI-job-itself claim was wrong. Re-promote to
469+
`[DONE]` once a real `editor-sanitizer` run is green.
470+
454471
- **SYS-W11-10** `[DONE]` `P2` — Wired one authoritative version source: a
455472
new root-level `VERSION` file (currently `0.1.0`, the existing value —
456473
deliberately not changed; deciding the actual first-release version number

0 commit comments

Comments
 (0)