QVAC-25104 fix: assert addons pin fabric's C++ runtime and narrow exports - #4519
Merged
Merged
Conversation
jpgaribotti
added
prebuilds
Contributor
License compliance — cleanNo new dependency license findings in this PR. Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
jpgaribotti
added a commit
that referenced
this pull request
Sep 16, 2026
…hips The six consumer ranges cannot move in the same commit as the bump after all. The pr-head-ts-checks jobs run a standalone npm install per package rather than through the pnpm workspace, so an unpublished ^0.16.0 fails resolution outright -- ETARGET, no matching version -- and took six jobs down with it. #4477 hit the same ordering constraint for the 0.15.0 floor: ranges move once the release is on npm, which is what #4519 does. The cost is that pnpm stops linking the workspace fabric into the six consumers, because 0.16.0 no longer satisfies ^0.15.0, so the regenerated lockfile records the published 0.15.0 for them. That is the same in-between state main sat in after #4481 and before #4477, and it is content-identical for those builds: the published 0.15.0 is the fabric they link today. Native CI is unaffected either way, since overlay-local-fabric stages this branch's own fabric into consumer node_modules when the fabric stack changes, which is what the prebuilds label drives. The window closes when #4519 lands.
jpgaribotti
added a commit
that referenced
this pull request
Sep 16, 2026
…hips The six consumer ranges cannot move in the same commit as the bump after all. The pr-head-ts-checks jobs run a standalone npm install per package rather than through the pnpm workspace, so an unpublished ^0.16.0 fails resolution outright -- ETARGET, no matching version -- and took six jobs down with it. #4477 hit the same ordering constraint for the 0.15.0 floor: ranges move once the release is on npm, which is what #4519 does. The cost is that pnpm stops linking the workspace fabric into the six consumers, because 0.16.0 no longer satisfies ^0.15.0, so the regenerated lockfile records the published 0.15.0 for them. That is the same in-between state main sat in after #4481 and before #4477, and it is content-identical for those builds: the published 0.15.0 is the fabric they link today. Native CI is unaffected either way, since overlay-local-fabric stages this branch's own fabric into consumer node_modules when the fabric stack changes, which is what the prebuilds label drives. The window closes when
jpgaribotti
added a commit
that referenced
this pull request
Sep 17, 2026
…ode (#4514) * QVAC-25104 fix: pin consumers' C++ runtime with a named ELF version node 0.15.0 gave fabric the C++ runtime to export, but nothing reached it. bare's executable links GNU libstdc++.so.6, so libstdc++ sits in the process' global lookup scope, which the dynamic linker searches before a dlopen'd module's own DT_NEEDED chain. A consumer linked with -nostdlib++ resolved __cxa_throw, the personality routine and the std:: typeinfo objects from libstdc++ while taking only the libc++-only names from fabric, and std::exception_ptr split down that seam: a captured exception was re-raised with a foreign exception class that GNU's personality routine can only match against catch (...). Every native error from load reached JS as "Unknown error", not only the three config-validation cases. Name the version node QVAC_FABRIC_ABI_1, so consumers record a DT_VERNEED that libstdc++ cannot satisfy. The node covers the whole export surface: an anonymous node cannot coexist with a named one, and the spliced C++ ABI block has to stay inside the same global list to keep its precedence over `local: *;`. It pins fabric's own internal references too, which were being interposed the same way. Publish QVAC_FABRIC_ABI_VERSION and QVAC_FABRIC_OWNS_CXX_RUNTIME through the package config, so a consumer's build can assert it pinned the runtime instead of trusting its link line, and can tell a deliberately shared libc++ (Android, ASan) from a fabric too old to pin at all. Configure fails if symbols.map and the declared node name drift apart. Consumers must be rebuilt, and a mixed pairing is worse than either half and silent: pinning fabric's internal references removes the accident that both sides previously resolved the runtime from libstdc++ and so agreed on one, so a consumer that is not rebuilt loses the typed catch it used to get -- and still loads, because an unversioned reference binds to a default-versioned definition. Ship as a minor; a caret on 0.x locks the minor, so ^0.15.0 keeps already-published consumers away from it. Verified on linux-x64 against a locally built fabric: a rebuilt llm-llamacpp pins 378 imports to QVAC_FABRIC_ABI_1, and the load-mode, repeat-penalty and device errors surface their real messages under bare. The addon-template half, which asserts the pin at build time and narrows addon exports to the bare C entry points, lands separately after this is released to npm. * QVAC-25104 chore: release @qvac/fabric v0.16.0 Folds the version bump into the fix PR so the release can be cut straight from a release-fabric-0.16.0 branch: release-merge-guard only requires package.json to match the branch version, and create-github-release reads the [0.16.0] section out of the changelog. Minor, not patch: a consumer that is not rebuilt against the version node loses a typed catch it used to get and still loads, so ^0.15.0 has to keep already-published consumers away from this release. The six in-tree consumer ranges move in the same commit. pnpm-workspace's linkWorkspacePackages resolves from the registry once the workspace version stops satisfying the range, so bumping fabric alone flips all six from link:../fabric to the published 0.15.0 -- which is both a lockfile change and a silent downgrade of what CI builds them against. Their own versions are untouched; nothing here releases a consumer. pnpm-lock.yaml is regenerated with the pinned pnpm 11.17.0 rather than hand-edited, so it also drops two orphaned @qvac/diffusion-cpp@0.23.0 entries that nothing has referenced since #4439 moved sdk and inference to the workspace copy. * QVAC-25104 fix: hold consumer fabric ranges at ^0.15.0 until 0.16.0 ships The six consumer ranges cannot move in the same commit as the bump after all. The pr-head-ts-checks jobs run a standalone npm install per package rather than through the pnpm workspace, so an unpublished ^0.16.0 fails resolution outright -- ETARGET, no matching version -- and took six jobs down with it. #4477 hit the same ordering constraint for the 0.15.0 floor: ranges move once the release is on npm, which is what #4519 does. The cost is that pnpm stops linking the workspace fabric into the six consumers, because 0.16.0 no longer satisfies ^0.15.0, so the regenerated lockfile records the published 0.15.0 for them. That is the same in-between state main sat in after #4481 and before #4477, and it is content-identical for those builds: the published 0.15.0 is the fabric they link today. Native CI is unaffected either way, since overlay-local-fabric stages this branch's own fabric into consumer node_modules when the fabric stack changes, which is what the prebuilds label drives. The window closes when * QVAC-25104 docs: correct the 0.16.0 changelog on when consumer ranges move
jpgaribotti
force-pushed
the
fix/qvac-25104-addon-template-cxx-pin
branch
from
September 17, 2026 08:38
bb5f885 to
8ded317
Compare
jpgaribotti
marked this pull request as ready for review
September 17, 2026 08:48
Contributor
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 more from Management, Team Lead, or Member. |
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
1 similar comment
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
…orts Consumer half of the fabric version-node change, and it lands after that is released to npm. Both halves of getting the pin wrong are silent. -nostdlib++ links cleanly whether or not fabric ends up providing the runtime, and a module that resolves it from the host's libstdc++ instead loads and runs -- it just stops matching typed catches on any exception that crossed the seam, surfacing as a package-level "Unknown error" a long way from the link that caused it. The built ELF is the only place the invariant is directly observable, so qvac_addon_finalize (template 0.2.0 -> 0.3.0) now runs assert-fabric-cxx-runtime.cmake on every fabric-linked module: no undefined C++ runtime symbol may lack a QVAC_FABRIC_ABI_1 requirement, excepting the __cxa_atexit / __cxa_finalize / _Unwind_* entries that glibc and libgcc_s own rather than fabric. Configure fails outright when the installed fabric publishes no QVAC_FABRIC_OWNS_CXX_RUNTIME, which is what catches a workspace that has fallen back to a registry fabric predating the node, and the check is skipped where fabric deliberately shares a libc++ by other means (Android, the ASan build). Also narrow addon modules to the bare C entry points with a version script, generalizing what packages/asr-ggml applies by hand: exports drop from 1985 to 2. bare loads modules RTLD_LOCAL, so the wider set was unreachable rather than actively harmful; declaring the interface makes the addon/fabric pair a closed unit exposing plain C by construction, rather than by the loader flags of whichever host loads it. Verified on linux-x64 against a locally built fabric: llm-llamacpp pins 378 imports to QVAC_FABRIC_ABI_1, exports 2 symbols, the assertion fails the build when the pin is removed, and the load-mode, repeat-penalty and device errors surface their real messages under bare.
…runtime @qvac/fabric 0.16.0 is published, so the consumer ranges can move to the version that actually pins the C++ runtime. ^0.15.0 -> ^0.16.0 across the npm_runtime roster restores link:../fabric in the lockfile as well: the 0.16.0 bump on main had pushed pnpm onto the published 0.15.0 for all six, because a caret on 0.x locks the minor. All six take a patch bump with a changelog entry, the same shape #4477 used for the 0.15.0 floor: every one already has its current version published, so none can fold the change into an unreleased entry, and a patch lets anyone tracking the current line pick the fix up without editing a range. model-fit joins them at 0.12.1 now that the 0.12.0 backmerge (#4511) has landed and main matches npm again; the fit-stub entry already sitting under its [Unreleased] heading stays there, being documentation and tests rather than shipped behaviour. The entries say plainly that the typed-catch behaviour their 0.15.0-floor predecessors described never took effect: with the runtime unpinned those handlers still saw a fabric throw as foreign. Changelogs are append-only here, so the correction is recorded in the new entry instead of rewritten into the old one.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
gianni-cor
reviewed
Sep 17, 2026
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
find_program() skips its search when the result variable already holds a value, and the empty string copied from an unset CMAKE_READELF counts, so the PATH fallback never ran: it reported that nothing was on PATH without having looked. Measured rather than reasoned from policy -- a bare find_program resolves the tool, the same call preceded by set(VAR "") returns nothing and writes no cache entry, and that holds under CMP0125 and CMP0126 at both OLD and NEW, so it is not tied to the 3.25 floor. Searching under QVAC_ADDON_READELF keeps the search reachable and gives the resolved tool a documented cache entry to override. The genuine absence path is unchanged: find_program yields -NOTFOUND, which is false, so the fatal error still fires when no readelf exists. Reported in review on #4519.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
gianni-cor
previously approved these changes
Sep 17, 2026
iancris
reviewed
Sep 17, 2026
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
iancris
reviewed
Sep 17, 2026
iancris
reviewed
Sep 17, 2026
GustavoA1604
previously approved these changes
Sep 17, 2026
…s config QVAC_FABRIC_OWNS_CXX_RUNTIME and QVAC_FABRIC_ABI_VERSION reach consumers through a config that fabric installs to share/qvac-fabric/cmake, a platform-shared path every prebuild leg writes and the artifact merge collapses to one file, so their values are whichever leg finished last. Only the two Linux legs of nine produce ON; a darwin leg publishes the variable empty and Android publishes OFF, and either one walks the old elseif into its silent skip -- no configure error, no POST_BUILD step, no message. The guard this PR exists to add would simply not be there, and which way it goes is re-rolled on every fabric release. The published 0.16.0 happens to carry ON. So the check now asks fabric's own module, which is per-triplet and cannot be the wrong platform's: __cxa_throw defined under a version node means assert against that node, defined without one means fail (a workspace that has fallen back to a registry fabric predating the scheme), and not defined at all means fabric shares libc++ through a shared library and there is nothing to pin to. That last case is the only legitimate skip and it is now observed rather than declared. Closes the mirror-image hole while here: a module that kept a runtime of its own imports nothing from fabric, and a check that only inspects imports passes it. It must now import at least one symbol under the node. The skip comment named Android as a reason, but the enclosing block is CMAKE_SYSTEM_NAME STREQUAL "Linux" and the NDK sets that to Android, so the ASan build was always the only case that could reach it. Reported in review on #4519.
The readelf parse was trusted to have produced something. It is now checked: keep the numbered symbol entries, drop the table header, and fail when none survive. Both files read here are dynamic objects that certainly have entries, so an empty parse means the output was not understood -- a readelf variant printing another format, a wrapper on PATH printing something else -- and every question below answers itself the permissive way. A stand-in printing "There is no dynamic symbol information in this file." and exiting 0, or printing nothing at all, made fabric look like it exports no runtime and the check skip with a status message; measured before and after. The module side of this was closed by the previous commit, which requires at least one import under the node, so a module whose runtime came from a bundled libc++.a or -static-libstdc++ -- contributing no undefined entries and, under --exclude-libs,ALL plus local: *, no visible defined ones either -- no longer arrives as silence. What was left was the fabric probe, where an unread file is indistinguishable from the one legitimate skip. Reported in review on #4519.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
1 similar comment
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
GustavoA1604
approved these changes
Sep 17, 2026
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
iancris
approved these changes
Sep 17, 2026
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
1 similar comment
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What problem does this PR solve?
-nostdlib++links cleanly whether or not fabric ends up providing the runtime, and a module that resolves it from the host's libstdc++ instead loads and runs — it just stops matching typed catches on any exception that crossed the seam, which surfaces as a package-level"Unknown error"a long way from the link that caused it. That is exactly how fabric 0.15.0 shipped a fix that never reached its consumers.pnpm-workspace.yamlsetslinkWorkspacePackages: trueand resolves from npm when the local version does not satisfy the range, so an addon can silently build against a published fabric that predates the node. This is not hypothetical — it is the statemainis in right now. QVAC-25104 fix: pin consumers' C++ runtime with a named ELF version node #4514 bumped fabric to0.16.0while the six consumers still asked for^0.15.0, and because a caret on a0.xversion locks the minor, all six stopped linking the workspace copy and resolved the published0.15.0instead.inference-addon-cpp. bare loads modulesRTLD_LOCAL, so those exports are unreachable rather than actively harmful, but it leaves the addon/fabric pair closed by the loader flags of whichever host loads it rather than by construction.📝 How does it solve it?
Template guards (
cmake/qvac-addon, template0.2.0→0.3.0):qvac_addon_finalizeruns the newassert-fabric-cxx-runtime.cmakeon every fabric-linked module as aPOST_BUILDstep.readelf --dyn-symsmust show no undefined__cxa_*/__gxx_personality_v0/_ZT*/_ZNSt*/ operator-new symbol without aQVAC_FABRIC_ABI_1requirement —__cxa_atexit,__cxa_finalizeand_Unwind_*excepted, since glibc and libgcc_s own those rather than fabric. The built ELF is the only place the invariant is directly observable.QVAC_FABRIC_OWNS_CXX_RUNTIME, which is what catches the registry fallback above. The assertion is skipped where fabric deliberately shares a libc++ by other means (Android'slibc++_shared.so, the ASan build'slibc++.so.1), which is what that flag distinguishes.addon-symbols.mapnarrows every ELF addon module tobare_*andnapi_*, generalising whatpackages/asr-ggml/symbols.mapapplies by hand for the same reason. Exports drop from 1985 to 2, so an addon and fabric form one closed unit exposing plain C by construction. Imports are unaffected:local: *;constrains what the module defines, not what it resolves from fabric.docs/architecture/ADDON-CMAKE-TEMPLATE.md, with the build-time guard recorded in the QIP's verification section.Consumer floor (second commit, now that
0.16.0is on npm):@qvac/fabric^0.15.0→^0.16.0for all sixnpm_runtimeconsumers in.github/fabric-consumers.json, which both satisfies the new configure guard and restoreslink:../fabricin the regeneratedpnpm-lock.yaml.0.15.0floor: each already has its current version published, so none can fold this into an unreleased entry, and a patch lets anyone tracking the current line pick the fix up without editing a range.model-fitjoins them at0.12.1now that its0.12.0backmerge (#4511) has landed andmainmatches npm again; the fit-stub entry already sitting under its[Unreleased]heading stays there, being documentation and tests rather than shipped behaviour.0.15.0-floor predecessors described never took effect, since with the runtime unpinned those handlers still saw a fabric throw as foreign. Changelogs are append-only here, so the correction is recorded in the new entry rather than rewritten into the old one.🧪 How was it tested?
Locally on linux-x64, with fabric built from #4514 and
llm-llamacppbuilt against it:QVAC_FABRIC_ABI_1.bare_register_module_v0,bare_get_module_name_v0), anddlsymon the plain names still resolves them, which is how bare locates a module.bare, the three previously failingllm-llamacppconfig scenarios and a genuine load failure all surface their real messages.@qvac/fabric@0.16.0resolves from npm, so the standalonenpm installinpr-head-ts-checksno longer hitsETARGETon the new ranges. Theprebuildslabel is still needed: the six consumers have to be rebuilt against0.16.0for the new guards to be exercised at all.Before this can merge
@qvac/fabric0.16.0released to npm@qvac/fabricto^0.16.0for the sixnpm_runtimeconsumers, each with a changelog entry and its own version bumppnpm-lock.yamlin the same commit, perCONTRIBUTING.mdmain— the rebased diff touchespackages/**, soon-pr-nxstill triggers (itspaths:filter would skip a template-only diff and merge it with no native CI at all)💥 Breaking Changes
No JS/TypeScript API change. Build-level: any target using
qvac_addon_import_fabric_cxx_runtimerequires a fabric that stampsQVAC_FABRIC_ABI_1, i.e.0.16.0or later, which the range bumps above enforce. Because the template is shared, the six ranges have to move together — a partial migration would leave the unmigrated addons failing the new configure guard.