diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 432b40efc..ad4985537 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -802,6 +802,15 @@ jobs: QT_QPA_PLATFORM: offscreen run: cmake --build --preset clang-tsan + # The same assertion linux-sanitizers and ladder-sanitizers make, for the + # same reason (morph#542): this job's whole claim is that kanban's stress + # test ran under ThreadSanitizer, and an uninstrumented binary makes that + # claim while proving nothing. Keyed on `__tsan_`, not `__asan_` — an + # ASan-only assertion on a TSan leg is itself a control that measures + # nothing. + - name: Every ctest binary is instrumented + run: bash scripts/check_sanitizer_instrumentation.sh build/clang-tsan tsan + # Every ladder ctest case only ever carries the "ladder"/"ladder-" # labels morph_add_rung() applies (cmake/morph_add_rung.cmake) — Catch2's # own tags ([kanban][stress][tsan]) are never translated into ctest @@ -814,6 +823,14 @@ jobs: - name: Test (kanban's TSan-tagged stress test only) env: QT_QPA_PLATFORM: offscreen + # The suppression file linux-sanitizers' Test step already passes, + # missing here (morph#542). Both entries are libstdc++ refcounts for + # a shared exception, decremented inside an uninstrumented + # libstdc++.so.6 (morph#476); the evidence is in cmake/tsan.supp. + # Without it this leg is a coin toss on a false positive that the + # other TSan leg already knows is one — and the failure mode of a + # known false positive is that the next real one is waved past. + TSAN_OPTIONS: suppressions=${{ github.workspace }}/cmake/tsan.supp run: ctest --preset clang-tsan -L ladder-kanban -R ThreadSanitizer --output-on-failure # ── Linux: Qt WebSocket backend build + tests ───────────────────────── @@ -1502,26 +1519,19 @@ jobs: # checked". That is not hypothetical — before the AF_SANITIZER blocks # landed in cmake/morph_add_rung.cmake and examples/common/CMakeLists.txt, # -DAF_SANITIZER=asan instrumented morph_tests and nothing under - # examples/. Assert the instrumentation is really there before trusting - # a green run. - - name: Verify rung binaries are actually instrumented + # examples/. + # + # This used to be a hand-rolled `nm | grep __asan_` over + # `examples/*/ladder_*_tests` alone, which is the same shape of blind + # spot one level up: it asserted the instrumentation of the binaries + # someone thought to name, and this job also builds tests/qt, + # examples/qt_tls_client and examples/concepts, none of which the glob + # reaches (morph#542). The shared script walks what ctest will actually + # run instead of a pattern, so a suite added tomorrow is covered by + # having been added. + - name: Every ctest binary is instrumented if: steps.filter.outputs.run == 'true' - run: | - set -euo pipefail - missing=0 - for exe in build/clang-asan/examples/*/ladder_*_tests; do - [ -x "$exe" ] || continue - if nm -C "$exe" 2>/dev/null | grep -q "__asan_"; then - echo "instrumented: $exe" - else - echo "NOT INSTRUMENTED: $exe" >&2 - missing=1 - fi - done - if [ "$missing" -ne 0 ]; then - echo "::error::ladder test binaries were built without ASan instrumentation" >&2 - exit 1 - fi + run: bash scripts/check_sanitizer_instrumentation.sh build/clang-asan asan # detect_leaks=0: LeakSanitizer reports allocations the Qt platform # plugins and QML engine intentionally keep for process lifetime, which @@ -1536,6 +1546,36 @@ jobs: UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 run: ctest --preset clang-asan -L ladder -LE stress --output-on-failure + # The Qt transport suites, which no sanitizer leg ran. They were not even + # instrumented until morph#542, and instrumenting a suite nothing runs + # would swap one vacuous control for another: this is the only leg that + # builds Qt under a sanitizer, so if these run anywhere it is here. + # linux-sanitizers deliberately keeps Qt out of its matrix (see its + # Configure comment), and kanban-tsan runs exactly one named stress test. + # + # Measured before adding this step, on clang 22 with these same options: + # all 67 cases across tests/qt, tests/net_qt_interop and + # qt_tls_example_runs pass clean under -fsanitize=address,undefined. This + # job's configure leaves MORPH_BUILD_NET off, so the filter below selects + # the 57 that exist here -- tests/qt plus the TLS example. + # + # `--no-tests=error` because `-R` over test names is exactly the filter + # that goes stale silently: a renamed TEST_CASE would otherwise leave + # ctest reporting success having run nothing (morph#466 is the same trap + # one level down). + - name: Test (the Qt transport suites, which no sanitizer leg ran) + if: steps.filter.outputs.run == 'true' + env: + QT_QPA_PLATFORM: offscreen + # detect_leaks=0 for the reason the ladder step above gives: Qt's + # platform plugins and QML engine keep allocations for process + # lifetime. The memory-error and UB checks stay fully on. + ASAN_OPTIONS: detect_leaks=0 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 + run: | + ctest --preset clang-asan --no-tests=error --output-on-failure \ + -R 'morph::qt::|qt_tls_example_runs' + # ── Linux: every optional feature enabled at once ───────────────────── # Every MORPH_BUILD_* option below is off by default, and until this job # existed no CI configuration turned any of them on — so several thousand diff --git a/.github/workflows/drift-guard.yml b/.github/workflows/drift-guard.yml index 39b7bf5fd..b4b8be8e5 100644 --- a/.github/workflows/drift-guard.yml +++ b/.github/workflows/drift-guard.yml @@ -81,7 +81,14 @@ jobs: name: Spec-citation, banned-terminology & CI-clang-pin lint runs-on: ubuntu-24.04 steps: + # fetch-depth: 0 for the spec-sync self-test at the end of this job: + # morph#560's two acceptance cases are named commits (9445bc04 and + # a72787ed), and under the default depth-1 checkout the self-test skips + # them rather than running them -- which would leave the one case that + # reproduces the reported defect never actually exercised in CI. - uses: actions/checkout@v4 + with: + fetch-depth: 0 # Same terms as rung-filter-lint below, and ci.yml's deprecation-lint and # test-type-name-lint: a lint gate nobody tests reports green whether or @@ -134,6 +141,19 @@ jobs: - name: Self-test the Catch2 test-name checker run: bash scripts/test_check_catch_test_names.sh + # ── The header <-> spec sync gate's own rule (#560) ────────────────── + # The gate runs in .github/workflows/spec-sync.yml, on a pull_request + # merge ref; its *rule* is scripts/check_spec_sync.sh, and a rule that is + # only ever exercised on the event it gates is one nobody has seen go + # red. It had not: include/morph/net/** was missing from its sub-domain + # list, so an interop-visible WebSocket behaviour change shipped with no + # documentation past a green tick (9445bc04). This drives the rule + # against that commit and against the one that documented it, plus every + # rejection and acceptance it claims -- here, where it costs a few + # seconds and no compilation. + - name: Self-test the header/spec sync gate + run: bash scripts/test_check_spec_sync.sh + - name: Check every test name selects itself as a Catch2 filter run: bash scripts/check_catch_test_names.sh diff --git a/.github/workflows/spec-sync.yml b/.github/workflows/spec-sync.yml index 9f45a96fb..2bcc612ea 100644 --- a/.github/workflows/spec-sync.yml +++ b/.github/workflows/spec-sync.yml @@ -1,8 +1,13 @@ name: Spec sync # Enforces that a change to a header sub-domain (include/morph//**) is -# accompanied by a change to the matching design-spec folder (docs/spec//**), -# so the specs cannot silently drift behind the code they describe. +# accompanied by a change to the docs that document that sub-domain, so the +# specs cannot silently drift behind the code they describe. +# +# Most sub-domains mirror their folder (docs/spec//**); morph::net and +# morph::render do not have one and name the files that document them instead. +# The mapping, and the requirement that every sub-domain be either mapped or +# explicitly exempt, are in scripts/check_spec_sync.sh (morph#560). # # The gate is skipped when the PR carries the label "no docs update" — a visible, # reviewable opt-out for changes that genuinely need no spec update. @@ -78,29 +83,13 @@ jobs: echo "$changed" echo "───────────────────" - # Sub-domains that have a mirrored spec folder. include/morph/detail/** - # and include/morph/qt/** have no specs and are intentionally exempt. - subdomains="core journal offline session forms util" - - missing="" - for sub in $subdomains; do - if echo "$changed" | grep -qE "^include/morph/$sub/"; then - if ! echo "$changed" | grep -qE "^docs/spec/$sub/"; then - missing="$missing $sub" - fi - fi - done - - if [ -n "$missing" ]; then - echo "" - echo "::error::Header changes without a matching spec update in:$missing" - for sub in $missing; do - echo " - include/morph/$sub/** changed but docs/spec/$sub/** did not." - done - echo "" - echo "Update the design spec(s) for the affected sub-domain(s), or add the" - echo "label 'no docs update' to this PR if no spec change is warranted." - exit 1 - fi - - echo "Spec sync OK: every touched header sub-domain has a matching spec change." + # The rule itself lives in scripts/check_spec_sync.sh, driven by the + # list of changed paths. It used to be an inline word list here -- + # `subdomains="core journal offline session forms util"` -- which + # left include/morph/net/** and include/morph/render/** silently + # exempt, so no change to the reference transport could make this + # gate red (morph#560). A rule in a script can be run against a list + # of paths, which is what lets scripts/test_check_spec_sync.sh drive + # it against the commits it must reject rather than asserting it + # works. + printf '%s\n' "$changed" | bash scripts/check_spec_sync.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index 5892c0106..82a05bf2a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -682,6 +682,25 @@ if(MORPH_BUILD_QT) target_link_libraries(morph_qt_impl PUBLIC morph_qt) set_target_properties(morph_qt_impl PROPERTIES AUTOMOC ON) + # The Qt WebSocket server and backend are the only *compiled* translation + # units in morph's own transport: everything else a Qt test exercises is + # header-only and so gets instrumented with the test target. Left out, a + # sanitizer leg watched every caller of this code and none of the code + # (morph#542). + # + # Safe here, unlike morph_test_main above, because every consumer of this + # archive is itself instrumented on a sanitizer preset -- the ladder + # targets, examples/common, examples/bank, tests/qt, + # tests/net_qt_interop and examples/qt_tls_client all carry an + # AF_SANITIZER block. apply_sanitizers()'s link options are PRIVATE and do + # not propagate out of a static library, so a consumer that did not would + # fail to link with `__asan_init` undefined rather than quietly lose its + # instrumentation. (The WASM spike links it too and is never built with + # AF_SANITIZER defined -- Emscripten has no such preset here.) + if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_qt_impl ${AF_SANITIZER}) + endif() + if(MORPH_BUILD_TESTS) add_subdirectory(tests/qt) endif() diff --git a/cmake/compiler_options.cmake b/cmake/compiler_options.cmake index f49ef6a29..7b38e390f 100644 --- a/cmake/compiler_options.cmake +++ b/cmake/compiler_options.cmake @@ -507,6 +507,20 @@ function(apply_sanitizers target mode) -fsanitize=undefined -fno-sanitize-recover=undefined -fno-omit-frame-pointer -g) target_link_options(${target} PRIVATE -fsanitize=undefined -fno-sanitize-recover=undefined) + else() + # No silent fall-through (morph#541). Without this arm, -DAF_SANITIZER=msan, + # =ASAN or a typo produced a *fully uninstrumented* build that configured, + # compiled and ran the entire suite green -- a sanitizer job reporting + # success having sanitized nothing, which is the failure mode this + # repository has hit most often. A configure-time error is the only place + # the mistake is still cheap: by build time every binary looks normal, and + # scripts/check_sanitizer_instrumentation.sh is the only thing that would + # notice, and only on the legs that run it. + message(FATAL_ERROR + "morph: apply_sanitizers(${target}): unknown sanitizer mode '${mode}'. " + "AF_SANITIZER must be one of: asan, tsan, ubsan (lower case). An " + "unrecognised value would otherwise leave this target uninstrumented " + "while the job that asked for a sanitizer reported success.") endif() endfunction() diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 04440594b..89a005c55 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -510,6 +510,12 @@ output is unchanged: no `x-layout`, `x-group`, `x-section`, or `x-colspan` key is emitted, and a renderer lays every field out exactly as it always has (flat, `x-order` order). +`groupKindName(GroupKind)` (`forms/layout.hpp`) is the one place the three +enumerators are spelled for the wire — `"section"`, `"tab"`, `"accordion"`, in +`x-layout.groups[].kind`. It is also the downgrade path: an out-of-range value +names itself `"section"`, matching the renderer's documented fallback for a +group kind it does not implement. + ```cpp // morph::forms::FieldGroup / FieldSpan / GroupKind — forms/layout.hpp. enum class GroupKind { Section, Tab, Accordion }; @@ -747,6 +753,12 @@ action that says nothing keeps the auto-submit default and its generated schema is byte-for-byte unchanged, so adding the emitter changed no shipped schema. Declaring `explicitSubmit = false` is the same statement as not declaring it. +The detection is the `HasExplicitSubmit` concept — `true` when `A` declares +an `explicitSubmit` member convertible to `bool`, the same shape as +`HasFormRules`. It answers only *whether the member is declared*; the +emitter reads its value afterwards, which is why `= false` and declaring +nothing produce the same schema. + Deriving the flag instead — from some "does this action mutate" predicate — is deliberately **not** done. No such predicate exists in `forms.hpp`, and adding one would flip the rendering of every existing generated form at once, which is @@ -1224,7 +1236,16 @@ case: `actionTypeId` is `ActionTraits::typeId()`; `wireField` is the member's reflected wire key (the same name `mergeSchemaExtras` iterates via `forEachNamedMember`); group/rule/step/menu indexes are the 0-based position -in their respective schema arrays. None of these keys are written into the +in their respective schema arrays. + +The field key is assembled from three named pieces rather than formatted in +one place, and a renderer reproducing this scheme in another language needs +all three: `fieldKeyStem(actionTypeId, wireField)` builds the +`.` stem, `fieldSlotName(FieldSlot)` spells the slot +suffix (`"label"`, `"help"`, `"placeholder"`), and `withSlot(stem, slot)` +joins them with a `.`. `fieldKey()` is `withSlot(fieldKeyStem(...), slot)` and +`explicitFieldKey()` is `withSlot(i18nKeyOverride, slot)` — which is why an +override replaces the stem and nothing else. None of these keys are written into the schema — a renderer derives them itself from data it already has (the schema plus the `actionType` label it is rendering under), so declaring nothing changes zero bytes of any schema. @@ -1413,21 +1434,30 @@ the client and the server evaluate identically from the same serialized form. ### The rule and condition kinds -| Factory | Meaning | `x-rules` `kind` | Also valid as a condition? | -|---|---|---|---| -| `requiredWhen(field, cond)` | `field` must be engaged when `cond` holds. | `"requiredWhen"` | no (only ranges over conditions itself) | -| `greater(a, b)` / `greaterOrEqual(a, b)` | `*a > *b` / `*a >= *b`. | `"greater"` / `"greaterOrEqual"` | yes | -| `less(a, b)` / `lessOrEqual(a, b)` | `*a < *b` / `*a <= *b`. | `"less"` / `"lessOrEqual"` | yes | -| `exactlyOneOf(f1, f2, ...)` | Exactly one listed field is engaged. | `"exactlyOneOf"` | no | -| `atLeastOneOf(f1, f2, ...)` | At least one listed field is engaged. | `"atLeastOneOf"` | no | -| `mutuallyExclusive(f1, f2, ...)` | At most one listed field is engaged. | `"mutuallyExclusive"` | no | -| `visibleWhen(field, cond)` | **Presentation:** `field` is shown only while `cond` holds. | `"visibleWhen"` | no | -| `readonlyWhen(field, cond)` | **Presentation:** `field` is editable only while `cond` does **not** hold. | `"readonlyWhen"` | no | -| `engaged(field)` / `notEngaged(field)` | `field` is / is not engaged. | `"engaged"` / `"notEngaged"` | yes (condition-only) | -| `equals(field, literal)` | `field`'s engaged value equals `literal`. | `"equals"` | yes (condition-only) | -| `andOf(cond1, cond2, ...)` | Every listed condition holds (boolean AND). | `"and"` | yes — also usable directly as a top-level rule | -| `orOf(cond1, cond2, ...)` | At least one listed condition holds (boolean OR). | `"or"` | yes — also usable directly as a top-level rule | -| `notOf(cond)` | The nested condition does **not** hold (boolean NOT). | `"not"` | yes — also usable directly as a top-level rule | +Every row names all three spellings of the same kind: the factory a C++ author +calls, the `kind` string the schema carries, and the `RuleKind` enumerator the +framework switches on. They are listed together because a reader emitting JSON +and a reader writing C++ read the same table, and the C++ capitalisation is not +derivable from the wire spelling by any rule stated anywhere. +`scripts/check_spec_citations.sh` (check 6) reads `ruleKindName()`'s switch and +requires each enumerator and its wire spelling to appear in one row here, so a +kind added to the enum cannot reach the wire undocumented. + +| Factory | Meaning | `x-rules` `kind` | `RuleKind` enumerator | Also valid as a condition? | +|---|---|---|---|---| +| `requiredWhen(field, cond)` | `field` must be engaged when `cond` holds. | `"requiredWhen"` | `RuleKind::RequiredWhen` | no (only ranges over conditions itself) | +| `greater(a, b)` / `greaterOrEqual(a, b)` | `*a > *b` / `*a >= *b`. | `"greater"` / `"greaterOrEqual"` | `RuleKind::Greater` / `RuleKind::GreaterOrEqual` | yes | +| `less(a, b)` / `lessOrEqual(a, b)` | `*a < *b` / `*a <= *b`. | `"less"` / `"lessOrEqual"` | `RuleKind::Less` / `RuleKind::LessOrEqual` | yes | +| `exactlyOneOf(f1, f2, ...)` | Exactly one listed field is engaged. | `"exactlyOneOf"` | `RuleKind::ExactlyOneOf` | no | +| `atLeastOneOf(f1, f2, ...)` | At least one listed field is engaged. | `"atLeastOneOf"` | `RuleKind::AtLeastOneOf` | no | +| `mutuallyExclusive(f1, f2, ...)` | At most one listed field is engaged. | `"mutuallyExclusive"` | `RuleKind::MutuallyExclusive` | no | +| `visibleWhen(field, cond)` | **Presentation:** `field` is shown only while `cond` holds. | `"visibleWhen"` | `RuleKind::VisibleWhen` | no | +| `readonlyWhen(field, cond)` | **Presentation:** `field` is editable only while `cond` does **not** hold. | `"readonlyWhen"` | `RuleKind::ReadonlyWhen` | no | +| `engaged(field)` / `notEngaged(field)` | `field` is / is not engaged. | `"engaged"` / `"notEngaged"` | `RuleKind::Engaged` / `RuleKind::NotEngaged` | yes (condition-only) | +| `equals(field, literal)` | `field`'s engaged value equals `literal`. | `"equals"` | `RuleKind::Equals` | yes (condition-only) | +| `andOf(cond1, cond2, ...)` | Every listed condition holds (boolean AND). | `"and"` | `RuleKind::And` | yes — also usable directly as a top-level rule | +| `orOf(cond1, cond2, ...)` | At least one listed condition holds (boolean OR). | `"or"` | `RuleKind::Or` | yes — also usable directly as a top-level rule | +| `notOf(cond)` | The nested condition does **not** hold (boolean NOT). | `"not"` | `RuleKind::Not` | yes — also usable directly as a top-level rule | `engaged`/`notEngaged`/`requiredWhen`/the membership rules accept any `EngageableField` — an `EmptyCapableField` (`Quantity`/`Choice`/`Timestamp`) or @@ -2065,6 +2095,7 @@ for the exhaustive tables and design rationale. | `EngageableField` | concept | `EmptyCapableField` or `std::optional<...>` — the broader "has an empty state" test the rule vocabulary uses. | | `Condition` | concept | `true` when `Cond` declares `static constexpr bool isCondition = true` — the admission test for a nested condition. `VisibleWhen`/`ReadonlyWhen`/`RequiredWhen` deliberately do not declare it; see [What may be a condition](#what-may-be-a-condition--the-condition-concept). | | `ComparableAgainstLiteral` | concept | `true` when a field of type `V` can be compared against a literal of type `L` at all. Constrains both `equals` overloads, so an incomparable pairing is an error at the call site. | +| `RuleLiteral` | concept | The closed set of literal types `equals` accepts: `std::int64_t`, `bool`, `std::string`, `math::Rational`, and a `FixedString` captured inline. It is what makes a literal serialise losslessly into `x-rules`; a type outside it is rejected at the call site rather than at schema-emission time. | | `RuleList` | class template | Holds an action's declared rules, in declaration order. Built by `ruleList(...)`; never constructed directly. | | `ruleList(rules...)` | function template | Composes rule/condition nodes into the `RuleList` an action assigns to `formRules`. | | `HasFormRules` | concept | `true` when `A` declares a `static constexpr formRules` member. | diff --git a/docs/spec/testing_charter.md b/docs/spec/testing_charter.md index f4f77d42a..c1bcf5283 100644 --- a/docs/spec/testing_charter.md +++ b/docs/spec/testing_charter.md @@ -67,6 +67,8 @@ floor means anything. | AddressSanitizer | Compiler instrumentation | `linux-sanitizers` (`clang-asan`), `ladder-sanitizers` | The CI job (nonzero exit on any diagnostic) | | UndefinedBehaviorSanitizer | Compiler instrumentation | `linux-sanitizers` (`clang-ubsan`), `ladder-sanitizers` | The CI job | | ThreadSanitizer | Compiler instrumentation | `linux-sanitizers` (`clang-tsan`), `kanban-tsan` | The CI job | +| A sanitizer leg can fail at all | `scripts/check_sanitizer_can_fail.sh` | `drift-guard` | The check: real undefined behaviour compiled with `apply_sanitizers()`'s own flags must make the process exit non-zero, and an unknown `AF_SANITIZER` must fail the configure (morph#541) | +| Every sanitized binary is really sanitized | `scripts/check_sanitizer_instrumentation.sh` | `linux-sanitizers`, `ladder-sanitizers`, `kanban-tsan` | The check: every binary `ctest` will run on a sanitizer leg must carry that sanitizer's runtime symbols (morph#542) | | Valgrind (memcheck) | Runtime instrumentation | `valgrind` CI job | The CI job | | Long-running / soak | `tests/soak/` | Local / on demand (`-DMORPH_BUILD_LOAD_TESTS=ON`; not a CI leg — see `docs/spec/testing_strategy.md`) | Those tests' own assertions over many cycles | | Compile-time contract checks | `tests/compile_checks/` | Every configure that reaches `tests/CMakeLists.txt` | `FATAL_ERROR` at configure time | diff --git a/docs/spec/testing_strategy.md b/docs/spec/testing_strategy.md index 5d67f1f3f..26771ee14 100644 --- a/docs/spec/testing_strategy.md +++ b/docs/spec/testing_strategy.md @@ -187,6 +187,14 @@ counter, `/proc/self/status`, and locally-owned atomic call counters), so their pass/fail signal never depends on how (or whether) a host application has wired up observability. +Both `morph_soak` and `morph_bench` are sanitizer-instrumented when +`AF_SANITIZER` is set (morph#542). They are opt-in, so no default sanitizer leg +pays for them; the reason to instrument them rather than exempt them is that +churn over thousands of cycles is exactly the shape of test whose finding is a +leak or a race and not a failed assertion. Under a sanitizer preset the +benchmark's published numbers are not comparable with an ordinary run's and are +not meant to be — there it is a correctness run over the dispatch path. + ## Load / latency benchmark (`tests/bench/`) Built only under `-DMORPH_BUILD_LOAD_TESTS=ON` alongside the soak tests (same diff --git a/examples/qt_tls_client/CMakeLists.txt b/examples/qt_tls_client/CMakeLists.txt index c87103210..766099997 100644 --- a/examples/qt_tls_client/CMakeLists.txt +++ b/examples/qt_tls_client/CMakeLists.txt @@ -12,6 +12,15 @@ target_link_libraries(morph_qt_tls_example PRIVATE morph_qt_impl) apply_warnings(morph_qt_tls_example) apply_bigobj(morph_qt_tls_example) +# This example is a ctest case (`qt_tls_example_runs`, below), so on a +# sanitizer preset it is a suite the job runs -- and, until morph#542, one it +# ran uninstrumented. It also links morph_qt_impl, which is instrumented on +# those presets; a consumer that is not would fail to link against it, the way +# morph_test_main's comment in the root CMakeLists.txt records. +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_qt_tls_example ${AF_SANITIZER}) +endif() + target_compile_definitions(morph_qt_tls_example PRIVATE QT_TLS_EXAMPLE_CERTS_DIR="${CMAKE_CURRENT_SOURCE_DIR}/certs" ) diff --git a/scripts/check_sanitizer_can_fail.sh b/scripts/check_sanitizer_can_fail.sh index 5c13d3e09..ba8d447fe 100755 --- a/scripts/check_sanitizer_can_fail.sh +++ b/scripts/check_sanitizer_can_fail.sh @@ -88,4 +88,32 @@ for mode in ubsan asan; do fi done +# ── An unrecognised mode must fail the configure, not the sanitizing ───────── +# +# apply_sanitizers() used to be `if asan / elseif tsan / elseif ubsan / endif` +# with no else arm, so `-DAF_SANITIZER=msan`, `=ASAN` or a typo produced a +# fully uninstrumented build that configured, compiled and ran the whole suite +# green (morph#541). That is the same "a leg that cannot fail" defect as a +# recovering UBSan, arriving through a different door: the job asks for a +# sanitizer, gets none, and reports success. +# +# Checked here rather than by reading the CMake, for this script's own stated +# reason: what has to hold is behavioural. +bad_build="${work}/build-badmode" +if cmake -S "${work}" -B "${bad_build}" \ + -DCMAKE_BUILD_TYPE=Debug \ + -DMORPH_COMPILER_OPTIONS="${repo_root}/cmake/compiler_options.cmake" \ + -DMORPH_SANITIZER_MODE="msan" > "${bad_build}.configure.log" 2>&1; then + echo "::error::check_sanitizer_can_fail: apply_sanitizers() accepted the unknown mode 'msan' and configured anyway -- that build would run the whole suite with no instrumentation at all and report success" + status=1 +else + if grep -q "unknown sanitizer mode" "${bad_build}.configure.log"; then + echo "check_sanitizer_can_fail: an unknown sanitizer mode fails the configure." + else + echo "::error::check_sanitizer_can_fail: the unknown-mode probe failed to configure, but not with apply_sanitizers()'s own diagnostic -- see ${bad_build}.configure.log" + cat "${bad_build}.configure.log" + status=1 + fi +fi + exit "${status}" diff --git a/scripts/check_spec_citations.sh b/scripts/check_spec_citations.sh index ce30365b0..662df85ae 100755 --- a/scripts/check_spec_citations.sh +++ b/scripts/check_spec_citations.sh @@ -2,7 +2,7 @@ # Usage: bash scripts/check_spec_citations.sh # # Prose-vs-manifest lint for the spec <-> code drift guard (see -# docs/spec/pinned_facts.toml and tests/test_pinned_facts.cpp). Two checks: +# docs/spec/pinned_facts.toml and tests/test_pinned_facts.cpp). Six checks: # # 1. Citation check: every pinned fact must still be *mentioned* in the # spec markdown file that documents it, so a spec cannot silently stop @@ -27,6 +27,11 @@ # there. Check 3 stops at the path, so a citation could point a reader at # a heading that was renamed or never written and still lint green # (morph#316). +# 6. forms key-vocabulary completeness: every `x-*` schema key, every +# `w-*`/`app-*` document key and every rule/condition kind the +# include/morph/forms/** headers emit must be a row in a +# docs/spec/forms/*.md table -- the same argument as check 4, on the other +# spec third-party implementers read as exhaustive (morph#554). # # This is a prose lint, not a value check: it does not parse # docs/spec/pinned_facts.toml or re-derive expected values (that is @@ -358,6 +363,92 @@ else echo "Section-citation check: ${sections_checked} cited sections scanned." fi +# --------------------------------------------------------------------------- +# 6. forms DSL key-vocabulary completeness +# --------------------------------------------------------------------------- +# morph::forms is a domain-specific language: a renderer that is not morph's own +# implements it from docs/spec/forms/, whose key table introduces itself as +# "Renderer contract: the schema key vocabulary". That claim is exactly the one +# docs/spec/core/wire.md made before morph#233 -- and morph's own Qt/QML +# renderer reads the same headers the schema generator does, so it cannot +# observe the *spec* disagreeing with them either. Only a check that reads both +# can (morph#554). +# +# Neither existing gate can see completeness. spec-sync.yml requires that +# *some* file under docs/spec/forms/ changed alongside include/morph/forms/**, +# which a typo fix satisfies; checks 1-3 above work outward from +# docs/spec/pinned_facts.toml, and a key nobody pinned is a key nobody checks. +# +# Three categories, each with its own floor for the reason check 4's comment +# records: a lumped total lets one category parse to nothing while the others +# carry it over the line. +forms_hpp="include/morph/forms/forms.hpp" +x_key_count=0 +doc_key_count=0 +rule_kind_count=0 + +forms_spec_files=(docs/spec/forms/*.md) + +if [ ! -f "$forms_hpp" ] || [ ! -f "${forms_spec_files[0]}" ]; then + echo "::error::forms key check: expected $forms_hpp and docs/spec/forms/*.md to exist" + fail=1 +else + # A table row for a key is `| \`x-key\` | ... |`. A key that is merely + # *mentioned* in prose is not documented as part of the vocabulary: the + # table is what a third-party renderer implements from. + for key in $(grep -rhoE '"x-[A-Za-z0-9]+"' include/morph/forms/ | tr -d '"' | sort -u); do + x_key_count=$((x_key_count + 1)) + grep -qE "^\| *\`${key}\`" "${forms_spec_files[@]}" || { + echo "::error file=docs/spec/forms/forms.md::schema key \`${key}\` is emitted by include/morph/forms/** but is not a row in any docs/spec/forms/ key table" + fail=1 + } + done + + # The document-level keys a workflow/app schema carries, same rule. + for key in $(grep -rhoE '"(w|app)-[A-Za-z0-9]+"' include/morph/forms/ | tr -d '"' | sort -u); do + doc_key_count=$((doc_key_count + 1)) + grep -qE "^\| *\`${key}\`" "${forms_spec_files[@]}" || { + echo "::error file=docs/spec/forms/forms.md::document key \`${key}\` is emitted by include/morph/forms/** but is not a row in any docs/spec/forms/ key table" + fail=1 + } + done + + # Rule/condition kinds, read off ruleKindName()'s switch -- the one place + # that maps a C++ enumerator to the wire spelling, so neither half can be + # invented here. The row must carry *both*: the wire spelling a JSON author + # emits and the C++ enumerator a C++ author writes. Requiring both is what + # closes the asymmetry morph#554 measured, where every membership and + # comparison construct appeared in the spec only under its JSON key + # spelling and a C++ reader had to guess the capitalisation. + while read -r enumerator kind; do + [ -n "$enumerator" ] || continue + rule_kind_count=$((rule_kind_count + 1)) + if ! grep -E '^\|' docs/spec/forms/forms.md \ + | grep -F "\`\"${kind}\"\`" \ + | grep -qF "RuleKind::${enumerator}"; then + echo "::error file=docs/spec/forms/forms.md::rule kind \`\"${kind}\"\` (RuleKind::${enumerator}) is not a table row naming both its wire spelling and its C++ enumerator" + fail=1 + fi + done < <( + awk '/ruleKindName\(RuleKind kind\)/, /^}/' "$forms_hpp" | awk ' + match($0, /case RuleKind::[A-Za-z]+:/) { + enumerator = substr($0, RSTART + 15, RLENGTH - 16) + next + } + enumerator != "" && match($0, /return "[A-Za-z]+";/) { + print enumerator, substr($0, RSTART + 8, RLENGTH - 10) + enumerator = "" + }' + ) + + if [ "$x_key_count" -lt 25 ] || [ "$doc_key_count" -lt 5 ] || [ "$rule_kind_count" -lt 16 ]; then + echo "::error::forms key check parsed ${x_key_count} x-keys (>=25), ${doc_key_count} document keys (>=5), ${rule_kind_count} rule kinds (>=16) -- a category came up short, so the check would pass while ignoring it" + fail=1 + else + echo "forms key check: ${x_key_count} schema keys, ${doc_key_count} document keys, ${rule_kind_count} rule kinds all documented." + fi +fi + if [ "$fail" -ne 0 ]; then echo "" echo "Prose lint failed. Either restore the missing citation or remove the" @@ -367,4 +458,4 @@ if [ "$fail" -ne 0 ]; then exit 1 fi -echo "Prose lint OK: every pinned fact is still cited; no banned terminology found; every cited path resolves; every cited section exists." +echo "Prose lint OK: every pinned fact is still cited; no banned terminology found; every cited path resolves; every cited section exists; every forms key and rule kind is documented." diff --git a/scripts/check_spec_sync.sh b/scripts/check_spec_sync.sh new file mode 100755 index 000000000..41a2a5a54 --- /dev/null +++ b/scripts/check_spec_sync.sh @@ -0,0 +1,217 @@ +#!/usr/bin/env bash +# Usage: git diff --name-only HEAD | bash scripts/check_spec_sync.sh +# +# The header <-> spec sync gate .github/workflows/spec-sync.yml enforces: a +# change to a header sub-domain must come with a change to the docs that +# document that sub-domain, so the specs cannot silently drift behind the code. +# +# It lives in a script rather than inline in the workflow so it can be driven +# against a list of paths -- which is what makes it testable +# (scripts/test_check_spec_sync.sh) and what makes "this gate goes red on that +# commit" a thing anyone can reproduce, rather than a claim about a YAML block +# that only ever runs on a pull_request event. +# +# ── Why the sub-domain list is a table and not a word list (morph#560) ─────── +# +# The gate used to read `subdomains="core journal offline session forms util"` +# and map `include/morph//` to `docs/spec//` by string substitution. +# Two sub-domains were missing from that list -- `net` and `render` -- and a +# sub-domain missing from it is not merely unchecked, it is indistinguishable +# from one that was deliberately exempted. Measured: commit 9445bc04 (morph#533) +# changed `WsFrameReader` to reject ten previously-accepted classes of +# WebSocket frame -- an interop-visible change to the reference transport -- +# and touched no documentation at all, with every gate green. +# +# The naive repair does not work, which is why this is a table. There is no +# `docs/spec/net/` folder: `morph::net` is documented in +# `docs/spec/core/backend.md` and `docs/spec/security.md`, and `morph::render` +# in `docs/spec/forms/forms.md`. Adding the word `net` to that list would have +# demanded a `docs/spec/net/` change that no correct pull request could make -- +# it would have failed morph#558, the pull request that documented the change +# properly. +# +# So each sub-domain names the doc paths that really document it, and -- the +# part that stops this recurring -- every directory under include/morph/ must +# appear either in that table or in the exempt list below. A new sub-domain is +# now a failure that names itself, instead of a silent exemption. +set -euo pipefail + +cd "$(git rev-parse --show-toplevel)" + +# ── The table: ||... ─────────────────── +# +# A sub-domain passes when the same change touches at least one path matching +# any of its regexes. Most are the 1:1 `docs/spec//` mirror; the two that +# are not carry the reason beside them. +spec_map=( + "core|^docs/spec/core/" + "journal|^docs/spec/journal/" + "offline|^docs/spec/offline/" + "session|^docs/spec/session/" + "forms|^docs/spec/forms/" + "util|^docs/spec/util/" + + # morph::net has no folder of its own. The SocketBackend/SocketServer API + # tables, the RFC 6455 frame-codec design-decision rows and the transport's + # limitations list live in docs/spec/core/backend.md; its TLS and + # resource-limit posture is docs/spec/security.md, "The Qt-free reference + # transport (`morph::net`)". `^docs/spec/net/` is listed first so that if + # the folder is ever created, it counts without a further edit here. + "net|^docs/spec/net/|^docs/spec/core/backend\.md\$|^docs/spec/security\.md\$" + + # morph::render is the renderer-side seam morph::forms is specified + # against: docs/spec/forms/forms.md documents resolveText(), + # normalizeLocaleNumber() and the TranslationProvider contract by name. + # `^docs/spec/render/` for the same forward-compatibility reason as net. + "render|^docs/spec/render/|^docs/spec/forms/" +) + +# Sub-domains with no spec of their own, deliberately exempt. +# +# detail -- implementation details (fixed_string, quantity_equation) that no +# spec describes as a public surface. +# qt -- the Qt backend/renderer. Its *behaviour* is specified through +# the core backend contract it realizes; there is no qt spec +# folder to require a change in. +# +# Anything not in this list and not in the table above is a hard failure, not +# an exemption: that silence is the defect morph#560 recorded. +exempt_subdomains="detail qt" + +changed="$(cat)" + +if [ -z "${changed//[[:space:]]/}" ]; then + echo "Spec sync OK: the change touches no files." + exit 0 +fi + +fail=0 + +mapped_subdomains="" +for entry in "${spec_map[@]}"; do + mapped_subdomains="${mapped_subdomains} ${entry%%|*}" +done +known_subdomains="${mapped_subdomains} ${exempt_subdomains}" + +is_known() { + case " ${known_subdomains} " in + *" $1 "*) return 0 ;; + *) return 1 ;; + esac +} + +# ── 1. Every sub-domain is classified ─────────────────────────────────────── +# +# Both the sub-domains that exist in the tree and the ones this change +# introduces: a pull request that adds include/morph// has to say which of +# the two lists above it belongs in, in the same change, rather than arriving +# exempt by omission. +subdomains_seen="" +for dir in include/morph/*/; do + [ -d "$dir" ] || continue + sub="$(basename "$dir")" + subdomains_seen="${subdomains_seen} ${sub}" +done +while IFS= read -r path; do + case "$path" in + include/morph/*/*) + sub="${path#include/morph/}" + sub="${sub%%/*}" + case " ${subdomains_seen} " in + *" ${sub} "*) ;; + *) subdomains_seen="${subdomains_seen} ${sub}" ;; + esac + ;; + esac +done <<< "$changed" + +classified=0 +for sub in ${subdomains_seen}; do + classified=$((classified + 1)) + if ! is_known "$sub"; then + echo "::error::include/morph/${sub}/ is neither in this gate's spec map nor in its exempt list, so every change to it is silently exempt -- the defect morph#560 recorded." + echo " Add \"${sub}|^docs/spec/\" to spec_map in scripts/check_spec_sync.sh," + echo " or add '${sub}' to exempt_subdomains with the reason it has no spec." + fail=1 + fi +done + +# A floor, for the same reason check_spec_citations.sh's table checks carry +# one: a glob that stopped matching would leave this loop classifying nothing +# and reporting success, which is the exact shape of failure this gate is for. +if [ "${classified}" -lt 8 ]; then + echo "::error::spec-sync: only ${classified} header sub-domain(s) found under include/morph/ -- expected at least 8; the scan is not seeing the tree and would pass vacuously." + fail=1 +fi + +# ── 2. Every mapping still points at something that exists ────────────────── +# +# A mapping whose target was renamed is worse than no mapping: it demands a +# change to a path no correct pull request can produce. Anchored file regexes +# are checked as files, directory prefixes as directories. +for entry in "${spec_map[@]}"; do + sub="${entry%%|*}" + rest="${entry#*|}" + reachable=0 + while [ -n "$rest" ]; do + pattern="${rest%%|*}" + if [ "$rest" = "$pattern" ]; then rest=""; else rest="${rest#*|}"; fi + literal="${pattern#^}" + case "$literal" in + *'$') + literal="${literal%'$'}" + literal="${literal//\\./.}" + [ -f "$literal" ] && reachable=1 + ;; + */) + [ -d "$literal" ] && reachable=1 + ;; + esac + done + if [ "${reachable}" -eq 0 ]; then + echo "::error::spec-sync: none of the doc paths mapped to include/morph/${sub}/ exist any more, so that mapping can never be satisfied. Fix the mapping in scripts/check_spec_sync.sh." + fail=1 + fi +done + +# ── 3. The gate itself ────────────────────────────────────────────────────── +missing="" +for entry in "${spec_map[@]}"; do + sub="${entry%%|*}" + rest="${entry#*|}" + if ! grep -qE "^include/morph/${sub}/" <<< "$changed"; then + continue + fi + satisfied=0 + patterns="" + while [ -n "$rest" ]; do + pattern="${rest%%|*}" + if [ "$rest" = "$pattern" ]; then rest=""; else rest="${rest#*|}"; fi + patterns="${patterns} ${pattern}" + if grep -qE "$pattern" <<< "$changed"; then + satisfied=1 + fi + done + if [ "${satisfied}" -eq 0 ]; then + missing="${missing}${sub}|${patterns# }"$'\n' + fail=1 + fi +done + +if [ -n "$missing" ]; then + echo "" + while IFS='|' read -r sub patterns; do + [ -n "$sub" ] || continue + echo "::error::include/morph/${sub}/** changed but none of its spec paths did." + echo " Accepted (any one of): ${patterns}" + done <<< "$missing" + echo "" + echo "Update the design spec(s) for the affected sub-domain(s), or add the" + echo "label 'no docs update' to this PR if no spec change is warranted." +fi + +if [ "${fail}" -ne 0 ]; then + exit 1 +fi + +echo "Spec sync OK: ${classified} sub-domain(s) classified; every touched header sub-domain has a matching spec change." diff --git a/scripts/test_check_spec_citations.sh b/scripts/test_check_spec_citations.sh index 1f77836b8..014033cb6 100755 --- a/scripts/test_check_spec_citations.sh +++ b/scripts/test_check_spec_citations.sh @@ -1,11 +1,13 @@ #!/usr/bin/env bash # Usage: bash scripts/test_check_spec_citations.sh # -# Self-test for the two gates in scripts/check_spec_citations.sh that were -# themselves added to close a citation family nothing could fail for: the +# Self-test for the checks in scripts/check_spec_citations.sh that were +# themselves added to close a family nothing could fail for: the # section-citation check (check 5, morph#316), which keeps a citation naming a # *section* of a markdown file pointing at a section that is really there, and -# pointing at an entry that still exists and is named by its slug. +# the forms key-vocabulary check (check 6, morph#554), which keeps +# docs/spec/forms/'s renderer-contract tables listing every schema key, +# document key and rule kind the headers actually emit. # # A lint gate nobody tests reports green whether or not it still detects # anything -- and this gate exists precisely because the surrounding script had @@ -176,6 +178,56 @@ expect_caught "the section-citation scan's own file glob going stale" \ break_section_scan \ 'section-citation check only scanned 0 cited sections' +# ── Each drift the forms key-vocabulary check claims to catch (check 6) ────── +# +# Same argument as check 4's, on the other spec third parties implement +# against: docs/spec/forms/ is the renderer contract for a DSL, and morph's own +# Qt/QML renderer reads the same headers the schema generator does, so nothing +# in the product can observe the spec falling behind the key vocabulary +# (morph#554). Each category is mutated on its own, since a lumped check would +# let one of them go blind behind the others. + +expect_caught "a documented x-* key losing its row in the spec" \ + "edit docs/spec/forms/forms.md -e '/^| \`x-placeholder\`/d'" \ + 'schema key `x-placeholder` is emitted by include/morph/forms/** but is not a row' + +expect_caught "a new x-* key reaching the schema with no row in the spec" \ + "printf '%s\n' '// emits \"x-undocumentedKey\" onto the property node' >> include/morph/forms/forms.hpp" \ + 'schema key `x-undocumentedKey` is emitted by include/morph/forms/** but is not a row' + +expect_caught "a document key losing its row in the spec" \ + "edit docs/spec/forms/workflows_navigation.md -e '/^| \`w-steps\`/d'" \ + 'document key `w-steps` is emitted by include/morph/forms/** but is not a row' + +# The asymmetry the check exists to keep closed: the wire spelling alone is not +# documentation for a C++ author, who cannot derive `AtLeastOneOf` from +# `atLeastOneOf` by any rule this spec states. +expect_caught "a rule kind row naming its wire spelling but not its C++ enumerator" \ + "edit docs/spec/forms/forms.md -e 's/\`RuleKind::AtLeastOneOf\`//'" \ + 'rule kind `"atLeastOneOf"` (RuleKind::AtLeastOneOf) is not a table row' + +# A kind added to the enum, and so to the wire, with nothing said about it. +expect_caught "a new rule kind added to ruleKindName() with no row in the spec" \ + "edit include/morph/forms/forms.hpp -e 's| case RuleKind::Engaged:\$| case RuleKind::Sideways:\n return \"sideways\";\n case RuleKind::Engaged:|'" \ + 'rule kind `"sideways"` (RuleKind::Sideways) is not a table row' + +# Vacuity guards, one per category: a scan that stopped matching would leave +# the category verifying nothing while the other two carried the check green -- +# which is precisely the failure check 4's per-category floors were added for. +break_x_key_scan() { + edit "$checker" -e 's/"x-\[A-Za-z0-9\]+"/"NO_SUCH_KEY_PREFIX-[A-Za-z0-9]+"/' +} +expect_caught "the x-* key scan matching nothing" \ + break_x_key_scan \ + 'parsed 0 x-keys' + +break_rule_kind_scan() { + edit "$checker" -e 's/ruleKindName\\(RuleKind kind\\)/noSuchFunctionName(RuleKind kind)/' +} +expect_caught "the rule-kind scan matching nothing" \ + break_rule_kind_scan \ + '0 rule kinds' + # ── The false positives it must not manufacture ────────────────────────────── # Headings across this tree carry an appended qualifier that citations of them # routinely drop -- `## Foo — bar` and `## Foo (bar)` are both cited as "Foo". @@ -194,4 +246,4 @@ if [ "$failures" -ne 0 ]; then exit 1 fi -printf '\nscripts/check_spec_citations.sh detects every section-citation drift it claims to.\n' +printf '\nscripts/check_spec_citations.sh detects every section-citation and forms-vocabulary drift it claims to.\n' diff --git a/scripts/test_check_spec_sync.sh b/scripts/test_check_spec_sync.sh new file mode 100755 index 000000000..84f384f28 --- /dev/null +++ b/scripts/test_check_spec_sync.sh @@ -0,0 +1,247 @@ +#!/usr/bin/env bash +# Usage: bash scripts/test_check_spec_sync.sh +# +# Self-test for scripts/check_spec_sync.sh, the header <-> spec sync gate. +# +# This gate's whole history is a gate that could not fail: `include/morph/net/` +# was absent from its sub-domain list, so no change to the reference transport +# could ever make it red, and commit 9445bc04 shipped an interop-visible +# WebSocket behaviour change with no documentation at all past a green tick +# (morph#560). A gate nobody drives against a tree it must reject is +# indistinguishable from that state, so every rejection it claims is +# reintroduced here, one at a time, and must be caught -- including the case +# where the gate's own scan goes blind. +# +# The two acceptance cases from morph#560 run against real history when it is +# present: the gate must reject commit 9445bc04 (the undocumented change) and +# accept a72787ed (morph#558, which documents it). They are skipped rather than +# failed in a shallow checkout, and the synthetic cases below cover the same +# two shapes unconditionally. +set -euo pipefail + +readonly repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly checker="${repo_root}/scripts/check_spec_sync.sh" + +failures=0 + +note() { printf 'ok: %s\n' "$*"; } +fail() { printf 'error: %s\n' "$*" >&2; failures=$((failures + 1)); } + +scratch="$(mktemp -d)" +trap 'rm -rf "$scratch"' EXIT + +# ── Cases driven by a path list alone, against this repository ────────────── + +# Each rejection must be a rejection *for the stated reason*: without the +# expected diagnostic, a checker that had started failing for an unrelated +# reason -- a syntax error, a missing file -- would read as fully working. +expect_rejected() { + local description="$1" paths="$2" expected="$3" output + if output="$(printf '%s\n' "$paths" | bash "$checker" 2>&1)"; then + fail "NOT caught: ${description} -- the gate passed a change it must reject" + return + fi + if printf '%s' "$output" | grep -qF "$expected"; then + note "rejected: ${description}" + else + fail "rejected for the WRONG reason: ${description} -- no diagnostic containing '${expected}':" + printf '%s\n' "$output" >&2 + fi +} + +expect_accepted() { + local description="$1" paths="$2" output + if output="$(printf '%s\n' "$paths" | bash "$checker" 2>&1)"; then + note "accepted: ${description}" + else + fail "FALSE POSITIVE: ${description} -- the gate rejected a change it must accept:" + printf '%s\n' "$output" >&2 + fi +} + +# The live gap morph#560 reported, in its minimal form. +expect_rejected "a morph::net header change with no spec change" \ + "include/morph/net/detail/ws_frame.hpp" \ + "include/morph/net/** changed but none of its spec paths did" + +# The sub-domain the same read found alongside net: morph::render was never in +# the list either, and is documented in docs/spec/forms/forms.md. +expect_rejected "a morph::render header change with no spec change" \ + "include/morph/render/locale_format.hpp" \ + "include/morph/render/** changed but none of its spec paths did" + +# The 1:1 mirrored sub-domains must still be gated -- the mapping table has to +# keep doing what the word list did. +expect_rejected "a core header change with no spec change" \ + "include/morph/core/backend.hpp" \ + "include/morph/core/** changed but none of its spec paths did" + +expect_rejected "a forms header change with no spec change" \ + "include/morph/forms/forms.hpp" \ + "include/morph/forms/** changed but none of its spec paths did" + +# A sub-domain that is in neither list arrives *exempt* under the old word +# list. That silence is the defect, so it is a failure that names itself. +expect_rejected "a new sub-domain that is in neither the map nor the exempt list" \ + "include/morph/newthing/thing.hpp" \ + "is neither in this gate's spec map nor in its exempt list" + +# ── The false positives it must not manufacture ───────────────────────────── + +# morph#558's shape: the spec that really documents morph::net, not a +# docs/spec/net/ folder that does not exist. A gate demanding the latter would +# have failed the pull request that fixed the documentation. +expect_accepted "a net change documented in docs/spec/core/backend.md" \ + "$(printf 'include/morph/net/detail/ws_frame.hpp\ndocs/spec/core/backend.md')" + +expect_accepted "a net change documented in docs/spec/security.md" \ + "$(printf 'include/morph/net/socket_server.hpp\ndocs/spec/security.md')" + +expect_accepted "a render change documented in docs/spec/forms/forms.md" \ + "$(printf 'include/morph/render/i18n.hpp\ndocs/spec/forms/forms.md')" + +expect_accepted "a core change documented in docs/spec/core/" \ + "$(printf 'include/morph/core/backend.hpp\ndocs/spec/core/backend.md')" + +# The two sub-domains that are deliberately exempt stay exempt. +expect_accepted "a qt and detail change with no spec change" \ + "$(printf 'include/morph/qt/qt_executor.hpp\ninclude/morph/detail/fixed_string.hpp')" + +expect_accepted "a change touching no headers at all" \ + "$(printf 'README.md\ntests/test_bridge.cpp')" + +# ── Cases that need a mutated tree ────────────────────────────────────────── +# +# The checker resolves its own root with `git rev-parse` and reads the tree for +# the sub-domain list and the mapping targets, so these run in a throwaway +# repository holding just the directory shape it looks at. +make_skeleton() { + local dest="$1" + rm -rf "$dest" + mkdir -p "$dest" + ( cd "$dest" \ + && mkdir -p include/morph/core include/morph/journal include/morph/offline \ + include/morph/session include/morph/forms include/morph/util \ + include/morph/net include/morph/render include/morph/detail include/morph/qt \ + docs/spec/core docs/spec/journal docs/spec/offline docs/spec/session \ + docs/spec/forms docs/spec/util \ + && : > docs/spec/core/backend.md \ + && : > docs/spec/security.md \ + && git init -q . ) +} + +run_in_tree() { + local tree="$1" paths="$2" + ( cd "$tree" && printf '%s\n' "$paths" | bash "$checker" 2>&1 ) +} + +expect_rejected_in_tree() { + local description="$1" tree="$2" paths="$3" expected="$4" output + if output="$(run_in_tree "$tree" "$paths")"; then + fail "NOT caught: ${description} -- the gate passed a tree it must reject" + return + fi + if printf '%s' "$output" | grep -qF "$expected"; then + note "rejected: ${description}" + else + fail "rejected for the WRONG reason: ${description} -- no diagnostic containing '${expected}':" + printf '%s\n' "$output" >&2 + fi +} + +# The skeleton itself must pass, or every mutation below would "pass" for the +# wrong reason. +make_skeleton "${scratch}/clean" +if output="$(run_in_tree "${scratch}/clean" "$(printf 'include/morph/net/x.hpp\ndocs/spec/security.md')")"; then + note "the unmutated skeleton tree is accepted" +else + fail "the unmutated skeleton tree was rejected:" + printf '%s\n' "$output" >&2 +fi + +# A mapping target that was renamed away. The gate would otherwise demand a +# change to a path no correct pull request can produce -- which is exactly what +# adding the bare word `net` to the old list would have done. +# +# The new name deliberately sits outside docs/spec/: spelling a +# docs/spec/.md path that does not exist would be caught by +# check_spec_citations.sh's own dangling-reference scan, which reads this file +# like any other. +make_skeleton "${scratch}/renamed" +mv "${scratch}/renamed/docs/spec/core/backend.md" "${scratch}/renamed/docs/renamed-away.md" +rm -f "${scratch}/renamed/docs/spec/security.md" +expect_rejected_in_tree "a mapping whose doc targets have all been renamed away" \ + "${scratch}/renamed" "README.md" \ + "none of the doc paths mapped to include/morph/net/ exist any more" + +# Vacuity guard on the gate's own scan. A glob that stopped matching would +# leave it classifying nothing and reporting success over every header in the +# tree -- the same shape of silence it exists to close. +make_skeleton "${scratch}/blind" +rm -rf "${scratch}/blind/include/morph" +expect_rejected_in_tree "the sub-domain scan finding no sub-domains at all" \ + "${scratch}/blind" "README.md" \ + "the scan is not seeing the tree and would pass vacuously" + +# ── morph#560's two acceptance cases ──────────────────────────────────────── +# +# The gate consumes one thing -- a list of changed paths -- so the two commits +# are pinned here as the exact lists `git diff --name-only ^ ` +# produces for them. Recorded rather than computed because neither commit is an +# ancestor of master (both live on origin/batch/533-535-536, which a branch +# deletion would take with it), and an acceptance case that silently stops +# running is the failure this whole gate is about. The live-history form runs +# too, whenever the objects are in the checkout. +readonly commit_9445bc04_paths='include/morph/net/detail/ws_frame.hpp +include/morph/net/socket_backend.hpp +include/morph/net/socket_server.hpp +tests/net/test_socket_backend.cpp +tests/net/test_socket_server.cpp +tests/net/test_ws_frame.cpp' + +readonly commit_a72787ed_paths='docs/spec/core/backend.md +docs/spec/security.md +docs/todo.md +include/morph/net/detail/tcp_socket.hpp +include/morph/net/detail/ws_frame.hpp +include/morph/net/socket_backend.hpp +include/morph/net/socket_server.hpp +scripts/branch_partial_allowlist.json +tests/net/test_socket_backend.cpp +tests/net/test_socket_server.cpp +tests/net/test_ws_frame.cpp' + +expect_rejected "commit 9445bc04's paths (the undocumented WebSocket behaviour change)" \ + "$commit_9445bc04_paths" \ + "include/morph/net/** changed but none of its spec paths did" + +expect_accepted "commit a72787ed's paths (morph#558, the same change with its docs)" \ + "$commit_a72787ed_paths" + +# When the objects are present, the recorded lists are checked against the +# commits themselves -- a fixture nobody reconciles with its source is how a +# test keeps passing about something that has changed. +for pinned in 9445bc04 a72787ed; do + if ! git -C "$repo_root" cat-file -e "${pinned}^{commit}" 2>/dev/null; then + note "skipped: commit ${pinned} is not in this checkout, so only its recorded path list ran" + continue + fi + live="$(git -C "$repo_root" diff --name-only "${pinned}^" "${pinned}")" + case "$pinned" in + 9445bc04) recorded="$commit_9445bc04_paths" ;; + a72787ed) recorded="$commit_a72787ed_paths" ;; + esac + if [ "$live" = "$recorded" ]; then + note "the recorded path list for ${pinned} still matches the commit" + else + fail "the recorded path list for ${pinned} no longer matches the commit:" + diff <(printf '%s\n' "$recorded") <(printf '%s\n' "$live") >&2 || true + fi +done + +if [ "$failures" -ne 0 ]; then + printf '\n%d self-test check(s) failed.\n' "$failures" >&2 + exit 1 +fi + +printf '\nscripts/check_spec_sync.sh rejects every drift it claims to, and none of the changes it must accept.\n' diff --git a/src/qt/forms/CMakeLists.txt b/src/qt/forms/CMakeLists.txt index b0dd24aba..f301eabe5 100644 --- a/src/qt/forms/CMakeLists.txt +++ b/src/qt/forms/CMakeLists.txt @@ -34,6 +34,16 @@ qt_add_qml_module(morph_forms_module target_link_libraries(morph_forms_module PUBLIC morph::morph Qt6::Quick Qt6::Qml) target_compile_features(morph_forms_module PUBLIC cxx_std_23) +# The shipped renderer's own compiled code (I18nCatalog) and the two suites +# below were never sanitizer-instrumented (morph#542): both suites are ctest +# cases, so a sanitizer preset with -DMORPH_BUILD_FORMS_QML=ON ran them and +# learned nothing. Same static-library caveat as morph_qt_impl in the root +# CMakeLists.txt -- every consumer here (the QML plugin, both test +# executables) is instrumented on the same preset. +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_forms_module ${AF_SANITIZER}) +endif() + # Qt Quick Test suite for the shipped renderer's own logic (schema parsing, # exact digit arithmetic, unit conversion, readiness) -- independent of any # app/demo. Later tasks add more tst_*.qml files here; -input (below) picks @@ -50,6 +60,10 @@ if(MORPH_BUILD_TESTS AND NOT EMSCRIPTEN) qt_add_executable(morph_forms_qml_tests tests/tst_main.cpp) target_link_libraries(morph_forms_qml_tests PRIVATE morph_forms_moduleplugin Qt6::QuickTest) + if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_forms_qml_tests ${AF_SANITIZER}) + endif() + # The shared x-rules corpus (morph#176) has two readers: this suite and # tests/test_forms_rule_corpus.cpp. Both are pointed at the one file from # CMake so neither can quietly grow its own copy. tst_main.cpp reads it and @@ -87,5 +101,9 @@ if(MORPH_BUILD_TESTS AND NOT EMSCRIPTEN) qt_add_executable(morph_forms_controller_core_tests tests/test_forms_controller_core.cpp) target_link_libraries(morph_forms_controller_core_tests PRIVATE morph::qt_forms Catch2::Catch2 morph_test_log_level) + + if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_forms_controller_core_tests ${AF_SANITIZER}) + endif() add_test(NAME forms_controller_core COMMAND morph_forms_controller_core_tests) endif() diff --git a/tests/bench/CMakeLists.txt b/tests/bench/CMakeLists.txt index 9b5b98731..92fc0e06b 100644 --- a/tests/bench/CMakeLists.txt +++ b/tests/bench/CMakeLists.txt @@ -8,5 +8,14 @@ target_include_directories(morph_bench PRIVATE ${CMAKE_SOURCE_DIR}/tests) target_compile_definitions(morph_bench PRIVATE BENCH_ARTIFACT_DIR="${CMAKE_CURRENT_BINARY_DIR}") apply_warnings(morph_bench) +# Instrumented under the same deliberate decision as tests/soak (morph#542). +# The numbers a sanitized benchmark reports are not comparable with an +# unsanitized one and are not meant to be: on a sanitizer preset this target is +# a correctness run over the dispatch path, and the throughput figures it +# publishes come from the ordinary presets. +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_bench ${AF_SANITIZER}) +endif() + include(Catch) catch_discover_tests(morph_bench DISCOVERY_MODE PRE_TEST PROPERTIES TIMEOUT 60 LABELS "bench") diff --git a/tests/net_qt_interop/CMakeLists.txt b/tests/net_qt_interop/CMakeLists.txt index e3716cc90..e1153503b 100644 --- a/tests/net_qt_interop/CMakeLists.txt +++ b/tests/net_qt_interop/CMakeLists.txt @@ -19,6 +19,13 @@ if(AF_COVERAGE) apply_coverage(morph_net_qt_interop_tests) endif() +# Likewise missing entirely (morph#542). This suite drives morph::net -- raw +# sockets, an I/O thread and a hand-rolled frame reader -- against Qt's +# transport, which is the part of the tree a sanitizer has most to say about. +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_net_qt_interop_tests ${AF_SANITIZER}) +endif() + include(Catch) get_target_property(_qt_core_dll Qt6::Core IMPORTED_LOCATION) diff --git a/tests/qt/CMakeLists.txt b/tests/qt/CMakeLists.txt index 0df49c861..c3c0846ab 100644 --- a/tests/qt/CMakeLists.txt +++ b/tests/qt/CMakeLists.txt @@ -10,6 +10,17 @@ add_executable(qt_test_client qt_test_client_main.cpp qt_test_models.hpp) target_link_libraries(qt_test_client PRIVATE morph_qt_impl) apply_warnings(qt_test_client) +# AF_SANITIZER, for the same reason these two carry apply_coverage(): the +# include/morph/qt code exercised across the process boundary only runs *here*, +# in the spawned child, so a sanitizer applied to the parent alone watches the +# half of the conversation that is not being tested (morph#542). An instrumented +# child reports on its own stderr and exits non-zero, which the parent's +# process-exit assertions already surface. +if(DEFINED AF_SANITIZER) + apply_sanitizers(qt_test_server ${AF_SANITIZER}) + apply_sanitizers(qt_test_client ${AF_SANITIZER}) +endif() + # TEST although neither is a ctest case: they are the other end of the # process-separation tests above, so the include/morph/qt code exercised across # that OS boundary only exists in *these* two binaries' coverage mapping. An @@ -52,6 +63,14 @@ if(AF_COVERAGE) apply_coverage(morph_qt_tests) endif() +# Was missing for the same reason and with the same consequence as the coverage +# block above: morph is header-only, so this target compiled its own +# uninstrumented copy of every include/morph/qt header while a job named for a +# sanitizer ran the suite and learned nothing from it (morph#542). +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_qt_tests ${AF_SANITIZER}) +endif() + # Make sure the helpers exist before the test binary tries to spawn them. add_dependencies(morph_qt_tests qt_test_server qt_test_client) diff --git a/tests/soak/CMakeLists.txt b/tests/soak/CMakeLists.txt index bc2c0916d..c26c8bdaf 100644 --- a/tests/soak/CMakeLists.txt +++ b/tests/soak/CMakeLists.txt @@ -14,5 +14,14 @@ target_link_libraries(morph_soak PRIVATE morph::morph morph_test_main) target_include_directories(morph_soak PRIVATE ${CMAKE_SOURCE_DIR}/tests) apply_warnings(morph_soak) +# morph#542 left the soak/bench decision explicit rather than assumed. Both are +# instrumented: they are opt-in (-DMORPH_BUILD_LOAD_TESTS=ON), so no default +# sanitizer leg pays for them, and switchBackend/reconnect churn over many +# cycles is exactly the shape of test whose finding is a leak or a race rather +# than a failed assertion -- which is the finding only a sanitizer reports. +if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_soak ${AF_SANITIZER}) +endif() + include(Catch) catch_discover_tests(morph_soak DISCOVERY_MODE PRE_TEST PROPERTIES TIMEOUT 300 LABELS "soak")