Skip to content

core: async attach hardening, client-side execute deadline, wire hardening - #42

Merged
Yaraslaut merged 20 commits into
masterfrom
framework/async-attach-and-execute-deadline
Aug 13, 2026
Merged

core: async attach hardening, client-side execute deadline, wire hardening#42
Yaraslaut merged 20 commits into
masterfrom
framework/async-attach-and-execute-deadline

Conversation

@Yaraslaut

Copy link
Copy Markdown
Member

What

Extracts every genuine morph framework change from the application-ladder branch onto its own branch against master, separate from the ladder/example work in #41. Nothing here depends on examples/, and nothing in examples/ depends on anything new here beyond what a normal consumer of these APIs would use.

Contents

Async attach/register hardening for shared and keyed models (Bridge/IBackend):

  • Adds an async register-or-attach/attach path for shared and keyed models (registerModelSharedAsync/attachModelAsync on IBackend, wired through Bridge::attachHandlerAsync/ensureBoundAsync, implemented in QtWebSocketBackend).
  • Fixes a reentrancy hazard where a backend that completes its callback inline (from inside the dispatch call itself, while the dispatching frame still holds _attachMtx) could have onDone invoked before the lock was released, plus a data race on HandlerBinding::contextKey/primary from an unsynchronized out-of-frame write.
  • Closes a switchBackend() staleness race: an in-flight async attach's reply could silently overwrite HandlerBinding state with an id meaningful only to a backend nothing uses anymore. Also adds exception-safety around attachModelAsync/registerModelSharedAsync's dispatch call and bookkeeping order.

Client-side execute deadline (Bridge::setExecuteDeadline):

  • New opt-in per-Bridge deadline that races a ClientTimeoutError against the real reply, independent of any server-side timeout.
  • Backed by a new TimeoutScheduler (extracted out of RemoteServer, where an equivalent mechanism already lived, into its own header so both client and server can share it), including a single-threaded-WASM build using the browser's setTimeout instead of a thread.

Wire hardening:

  • ActionTraits<A>::toJson/resultToJson now escape ASCII control bytes the same way wire::encode already does — previously an action or result body containing one produced invalid JSON the peer's own reader rejected.

morph::units::toString() (include/morph/util/quantity.hpp):

  • Exposes Quantity<U, Dec>'s existing std::formatter rendering logic as a plain function, and has the formatter delegate to it. Works around a libc++ limitation (seen on Emscripten's bundled version) recognizing std::formatter partial specializations parameterized over an auto NTTP for std::format's compile-time formattability check.

Test coverage: tests/test_async_registration.cpp, tests/test_client_execute_deadline.cpp, tests/test_wire_hardening.cpp, tests/qt/test_qt_websocket.cpp (4 new round-trip cases against a real server), tests/test_completion.cpp (documents CompletionState<T>::attachOnError's single-slot behavior at its source).

Build/portability:

  • Windows: build-wide NOMINMAX/WIN32_LEAN_AND_MEAN, _fileno for the Windows CRT, quint16{0} disambiguation for MSVC overload resolution.
  • Clang -Weverything: suppress -Wc++20-compat/-Wdisabled-macro-expansion, probe three suppressions via check_cxx_compiler_flag() instead of assuming an older bundled Clang has them, fix a @tparam/@param doc-vs-declaration mismatch in forms.hpp/qt_websocket_backend.hpp, drop an unused this capture in Bridge::executeVia.
  • GCC: suppress -Wmissing-field-initializers (GCC's name for the same deliberately-partial designated-init pattern Clang's narrower -Wmissing-designated-field-initializers already covers).

Every hunk here was mechanically extracted from application-ladder's commits (several of which mixed framework and ladder-only changes in one commit); each commit message says exactly what was left behind and why.

Verification

Configured and built standalone (-DMORPH_BUILD_LADDER=OFF -DMORPH_BUILD_EXAMPLES=OFF, no dependency on the ladder's Lightweight/ODBC fetch):

  • morph_tests: 867 test cases / 8622 assertions, all pass.
  • morph_qt_tests (offscreen): 63 test cases / 428 assertions, all pass.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
include/morph/core/bridge.hpp 97.28% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

Yaraslaut pushed a commit that referenced this pull request Aug 11, 2026
…dispositions

Filed one GitHub issue per genuinely open finding (26 total: 003, 005-009,
011-014, 017, 019-027, 029-032, 034, 036), each with a reproducer or a
desired-behavior/suggested-fix-direction section, cross-referenced back
into its finding .md file via a new `issue:` frontmatter line.

While drafting these, re-verified every finding's disposition against the
current tree instead of trusting stale frontmatter, and found five whose
`disposition: open`/`fix-scheduled` no longer matched reality -- the
underlying fix had already landed on this branch after the finding was
filed, but the frontmatter was never updated:

- 001 (async shared/keyed attach): registerModelSharedAsync/
  attachModelAsync exist and are wired through Bridge -- fixed.
- 002 (client execute deadline): Bridge::setExecuteDeadline exists,
  tested -- fixed.
- 004 (fault-injection wire proxy / deterministic strand interleaver):
  both exist under examples/common/testkit/ -- fixed.
- 028 (ladder tests inherit Lightweight's -Weverything warnings): fixed
  by 18d9438 and e737a77 (SYSTEM include demotion).
- 033 (BackendRig switch missing default): fixed by e737a77 (added the
  default: label).

No issue was filed for these five, nor for 010/015/016/018
(documented-limitation: intentional design tradeoffs, not bugs) or 035
(already tracked via PR #42's framework fix). 36 findings total = 26
issues + 10 correctly left without one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut and others added 16 commits August 12, 2026 23:05
wire::encode has escaped ASCII control bytes since the fuzz-found envelope
bug, but an execute envelope's `body` is not written by encode at all — it
is produced separately by ActionTraits<A>::toJson / resultToJson, which
wrote with plain glz::write_json and so reproduced the identical gap for
every string field of every action and result: invalid JSON that the peer's
own reader rejects, and — alongside an escaped character — silent
corruption into two 0x00 bytes by glaze's chunked write path.

Found from the other end, by the application ladder's rung 1 replaying
tests/fuzz/findings/ as paste content. Fixed with the same instrument one
layer down: model::detail::EscapingWriteOpts, deliberately duplicating
wire::detail::EscapingWriteOpts rather than making the model layer depend on
the transport layer's header for a four-line option struct.

Regression tests land as "Bug G" in test_wire_hardening.cpp, alongside the
envelope-level cases they mirror; all four fail if the write options are
reverted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HffixfknmXdAjnM5hSGRBb
…alone

Review follow-ups on the client-side execute deadline:

- Add include/morph/core/timeout_scheduler.hpp to the morph target's
  FILE_SET HEADERS list, so VERIFY_INTERFACE_HEADER_SETS compiles it
  standalone like every other public header.
- Add a test that the disarm actually releases the scheduler entry.
  CompletionState is first-write-wins, so a stray timer firing after an
  on-time reply is invisible at the value level; what the disarm buys is
  lifetime. The new case watches the CompletionState through a weak_ptr
  and fails if the pending timer still pins it. Correct the third case's
  trailing comment, which claimed a coverage that did not exist.
- Drop NeverRepliesBackend::liveCompletions (written, never read).
- Drop <map>/<thread> from remote.hpp, dead since TimeoutScheduler moved.
- Merge the duplicated backend.md entry in completion.md's cross-references.
- Note in setExecuteDeadline's docs that the clock starts before the
  backend's own execute() dispatch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HffixfknmXdAjnM5hSGRBb
…frame

`Bridge::attachHandlerAsync`/`ensureBoundAsync` promise to release
`_attachMtx` before calling `onDone`. That held for every path they
control, but not for a backend that completes its callback *inline*, from
inside `attachModelAsync`/`registerModelSharedAsync` itself, while the
dispatching frame is still holding the lock around the dispatch call --
which is exactly what `QtWebSocketBackend` does on its `!_connected`
branch.

An inline callback now parks its outcome in a `detail::AsyncDispatchHandoff`
and returns without acting; the dispatching frame claims it once the
dispatch call has returned, publishes it under the lock it already owns,
releases the lock, and only then reports. A small mutex in the handoff
makes the window race-free even against a backend that replies from
another thread while its dispatch call is still on this stack, and keeps
`onDone` invoked exactly once on every interleaving.

With the inline case structurally excluded, `attachHandlerAsync`'s
out-of-frame success callback can now take `_attachMtx` to publish
`HandlerBinding::contextKey`/`primary` -- two plain `std::string`s that
five other sites read under that lock, and that this callback previously
wrote unsynchronized (a data race, not merely a stale read).
`ensureBoundAsync`'s callback publishes only the atomic `currentId` and
needs no lock.

Also documents the in-flight-attach dedup gap on both methods (concurrent
same-key calls before the first reply are not coalesced and can leak one
bounded, connection-scoped server-side attach reference), and routes a
throwing key extraction in `execute()`'s payload-keyed branch through the
returned `Completion` instead of out of the call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HffixfknmXdAjnM5hSGRBb
… WASM

The final whole-branch review's I1, confirmed real by reading the chain it
cites: an `EventPoller`'s constructor calling `Bridge::setExecuteDeadline`
unconditionally lazily constructs a `TimeoutScheduler` (bridge.hpp), and
that constructor spawned a `std::thread` -- while single-threaded
Emscripten builds (no `-pthread`) have no working `pthread_create`.
Emscripten's non-pthread `pthread_create` stub fails, so libc++ throws
`std::system_error` from that constructor on every use.

Fixed rather than degraded: under
`__EMSCRIPTEN__ && !__EMSCRIPTEN_PTHREADS__` the same public API
(`schedule`/`cancel`/`Handle`) is built on `emscripten_async_call` -- the
browser's `setTimeout` -- and fires on the main thread. Deadlines still
fire; `ClientTimeoutError` still races the real reply. Two documented
behavioural differences: callbacks are never concurrent with the caller,
and `cancel()` releases the callback immediately but lets the underlying
browser timer elapse harmlessly instead of clearing it. Pending timers
hold a `weak_ptr` to the scheduler's state, so one that outlives its
scheduler returns without touching freed storage.

Honest scope: no Emscripten toolchain exists in this repository, so
neither the original hazard nor this fix has been observed on a real WASM
build -- stated as such in the header and in docs/spec/core/completion.md.
What *was* verified locally: the browser branch compiles warning-free
under a stubbed `emscripten.h` with `-D__EMSCRIPTEN__`, and against a
queued stub it fires once, honours `cancel()`, and drops pending timers
safely when the scheduler is destroyed.

This cherry-pick from application-ladder drops the original commit's
examples/common/gui/event_poller.hpp hunk (the actual EventPoller consumer
that motivated this fix) -- that file is ladder-only and doesn't exist on
this framework-only branch. Only the general-purpose TimeoutScheduler
change and its spec update are included here; the ladder branch retains
its own EventPoller fix built on top of this.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aps in

async keyed/shared attach

Findings from a /code-review max pass over the application-ladder branch,
fixed one round:

attachHandlerAsync's and ensureBoundAsync's out-of-frame success callbacks
published an async reply into HandlerBinding state (currentId/primary/
contextKey) with no check that the backend which produced it was still
Bridge's active backend. registerHandlerImpl already has the right guard
(weakBackend + pinned != loadBackend()) for its own synchronous-registration
callback; the two async-attach paths added later never got the same
treatment. A switchBackend() racing an in-flight attach could silently
overwrite a binding with an id only meaningful on the backend nothing uses
any more. Fixed by applying the identical guard to both callbacks -- but
unlike registerHandlerImpl's fire-and-forget re-registration, a real
execute() call is synchronously waiting on attachHandlerAsync/
ensureBoundAsync's onDone, so a stale reply now reports failure through it
instead of being silently dropped (which would otherwise hang the caller
forever). Two new regression tests in test_async_registration.cpp exercise
this against attachHandlerAsync (empirically confirmed to fail without the
fix: the stale reply corrupted handler.primary() and the subsequent action
against the new backend never re-dispatched) and ensureBoundAsync.

The backend dispatch call itself (attachModelAsync/registerModelSharedAsync)
had no try/catch, unlike the synchronous fallback a few lines below --
QtWebSocketBackend's real implementation calls wire::encode() directly,
documented to throw on serialization failure, which would otherwise escape
BridgeHandler::execute() as a raw exception and break its documented
never-throws contract. Fixed with a try/catch converting the exception into
onDone's failure path. Separately, those same three QtWebSocketBackend
methods (registerModelAsync/registerModelSharedAsync/attachModelAsync)
inserted their pending-registration bookkeeping *before* calling encode(),
so a throw there would orphan that entry forever, waiting for a reply to a
message never sent -- fixed by encoding first and only inserting once
encoding succeeds.

attachHandlerAsync's success-callback string writes (contextKey/primary)
were unguarded too, unlike the synchronous fallback that wraps the same
writes so onDone still fires on a throwing assignment. Fixed the same way,
in both the out-of-frame callback and the inline-claimed-outcome branch.

Bridge::executeVia's client-side-deadline cancel() call was guarded only by
a check-then-use `!alive.expired()` test -- proving Bridge's own liveness
token hadn't been destroyed at that instant, not that ~Bridge() (which can
run concurrently on another thread per docs/spec/core/bridge.md) wouldn't
complete ~TimeoutScheduler()'s thread-join between the check and the
cancel() call a few instructions later. ~Bridge()'s own body never acquires
_executeDeadlineMtx, so that lock doesn't close the gap either. Fixed by
changing _timeoutScheduler from unique_ptr to shared_ptr and having the
completion callbacks hold their own shared_ptr copy (captured under the same
lock as schedule()), so cancel() is always called on a scheduler that is
provably still alive by construction, not by timing luck; also wrapped the
call in try/catch, matching the pattern already used for the value-
forwarding code beside it.

timeout_scheduler.hpp: documented (not "fixed" -- emscripten_async_call's
int parameter is a hard platform constraint) that the WASM build's delay
saturates at ~24.85 days where the threaded build honors the full
std::chrono::milliseconds range.

Verified: full ladder 311/311 (was 307 before this branch's own unrelated
test additions), morph_tests 868/868, morph_qt_tests 63/63 (one transient
waitForConnected(2000) flake observed and reproduced as pre-existing/
unrelated -- 4/5 clean reruns, a real-network reconnect test untouched by
any of these changes).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HffixfknmXdAjnM5hSGRBb
`registerModelSharedAsync`/`attachModelAsync` had no direct test. Four
round-trip cases in the existing RemoteServer rig: a shared register that
lands in the instance directory and whose repeat joins the same instance,
an attach that joins a seeded instance and then re-points to another, the
empty-primary degrade-to-private branch, and the `!_connected` branch that
reports `onError` inline.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HffixfknmXdAjnM5hSGRBb
…ingle-slot)

Adds a direct test that CompletionState<T>::attachOnError keeps only the
most recently attached handler: attaching .onError() twice to the same
pending Completion<int> and asserting only the second handler fires. This
documents the single-slot mechanism at its source in the core test suite.

Extracted from application-ladder's 46c08f3, which paired this with a
ladder-specific test (examples/common/testkit/test_presenter.cpp,
exercising Presenter::track()'s onErr forwarding) — that half stays on
the ladder branch since Presenter doesn't exist here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… auto-NTTP formattability gap

Emscripten's bundled libc++ has a known limitation recognising a
std::formatter partial specialization parameterized over an `auto`
non-type template parameter (the unit enumerator U here) for
std::format's compile-time formattability check: `std::format("{}",
someQuantity)` fails to compile there with "the supplied type is not
formattable", even though the specialization is valid and the identical
call compiles and runs correctly on every other toolchain (native
Windows/Linux).

Adds morph::units::toString(Quantity<U, Dec>) -- the formatter's own
rendering logic, exposed as a plain function -- and has the formatter
delegate to it instead of duplicating the logic. A caller that hits the
libc++ gap can call toString() directly, bypassing std::format's buggy
compile-time trait check entirely. Produces byte-identical output to the
std::formatter path, so this is a non-behavioral refactor for every
existing caller.

Updates docs/spec/util/quantity_type.md to document the new symbol and
the delegation, closing the header/spec sync gate this trips.

Extracted from application-ladder's 2ddbb85 + cc2ae7b, which paired this
with ladder call sites (pastebin/bookmarks/polls QML bridges) switching
from std::format to toString() -- those stay on the ladder branch, which
is where the WASM build that actually exercises this gap lives.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…g has them

Emscripten's bundled clang (pinned by EMSDK_VERSION in the WASM
workflows) is older than the Linux/Windows clang this project otherwise
builds with, and rejects -Wno-nrvo, -Wno-unsafe-buffer-usage-in-libc-call,
and -Wno-c2y-extensions outright with "unknown warning option" --
turning three suppression flags into the exact -Werror failures they
exist to silence, the moment a WASM configure actually compiles a
target that reaches apply_warnings() (morph_ladder_gui, the first one
in build order, once the earlier morph_add_rung.cmake and wasm_spike
fixes let the build get this far).

check_cxx_compiler_flag() gates each of the three behind a genex now,
probed once per configure and cached like every other CMake compiler
check -- self-maintaining across future clang releases on either
toolchain, unlike a hard-coded version cutoff.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two more Clang -Weverything diagnostics the WASM leg's Emscripten-
bundled clang fires that the Linux/Windows clang builds never have,
now that the previous three fixes let morph_ladder_gui's actual source
files reach the compiler:

- -Wc++20-compat: the narrower sibling of the already-suppressed
  -Wpre-c++20-compat, covering consteval and implicit `typename` in
  alias templates specifically. Same "we target C++23" rationale as
  every other -Wno-pre-c++*-compat entry already in this list --
  model_key.hpp, quantity.hpp, forms.hpp and bridge.hpp all use both
  constructs deliberately.
- -Wdisabled-macro-expansion: Emscripten's sysroot stdio.h defines
  `#define stderr (stderr)` (a legal self-referential macro), which
  logger.hpp's `std::println(stderr, ...)` trips on. The macro is the
  platform's, not this codebase's -- nothing to fix in logger.hpp.

Both are long-standing, stable Clang flags (unlike the three from the
previous commit), so added unconditionally rather than behind a
check_cxx_compiler_flag() probe.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reached morph_ladder_gui/morph_ladder_app's actual source files for the
first time (steps 30-52/87) once the previous suppressions cleared;
three new -Werror failures surfaced there:

- forms.hpp: `@tparam N` on isLiteralString<LiteralString<N>>'s
  variable-template partial specialization trips -Wdocumentation --
  this Clang doesn't recognize @tparam as attached to a specialization
  the way it does the primary template just above it. Folded the
  parameter description into @brief prose instead of documenting a
  tag Clang won't accept, no information lost.
- qt_websocket_backend.hpp: `@param tls` documented a constructor
  parameter that is #ifndef QT_NO_SSL-only (a WASM build is always
  QT_NO_SSL) -- true doc/declaration mismatch on this specific
  compile. Wrapped the doc line in the identical #ifndef QT_NO_SSL
  guard as the parameter itself.
- compiler_options.cmake: -Wmissing-designated-field-initializers
  flags morph::session::Context{.principal = principal} for not also
  setting .token -- exactly this codebase's normal way to construct a
  DTO/config-style aggregate with everything else left at its member
  default (55+ occurrences of the same pattern across the ladder
  rungs). Added as a fifth check_cxx_compiler_flag()-gated suppression,
  same rationale as the four already added for this WASM leg.

Verified locally: ladder_pastebin_tests, ladder_bookmarks_tests,
ladder_polls_tests, ladder_common_tests all rebuild clean and pass
(bookmarks' one failure is the known pre-existing Windows temp-file-
lock flake in test_app.cpp).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t DTOs

GCC's -Wextra implies -Wmissing-field-initializers, which -- unlike
Clang's narrower -Wmissing-designated-field-initializers, already
suppressed for the identical reason -- fires on every field a designated
initializer leaves unset, flagging this codebase's normal way to
construct a DTO/config-style aggregate with everything else left at its
member default (morph::session::Context{.principal = ...} and its many
siblings) as a defect, one diagnostic per omitted field.

Extracted from application-ladder's dcd411c, which paired this with
ladder-only fixes (bookmarks TokenIssuer call sites, a ci.yml Qt version
gap) that stay on the ladder branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Define NOMINMAX/WIN32_LEAN_AND_MEAN build-wide, before any subdirectory
  is added, so any future <Windows.h>-including dependency's min/max
  macros can't leak into a translation unit that later calls
  std::min/std::max/std::numeric_limits<T>::min().
- tests/qt/qt_test_server_main.cpp: use _fileno instead of fileno for the
  Windows CRT (POSIX fileno isn't declared there).
- tests/qt/test_qt_websocket.cpp: disambiguate 0-literal overload
  resolution under MSVC for QtWebSocketServer's port parameter
  (quint16{0} instead of a bare 0, which MSVC can resolve to the wrong
  overload).

Extracted from application-ladder's 1043b7c, which paired these with
ladder-only fixes (Lightweight ORM fetch config, vcpkg.json additions,
[[nodiscard]] casts and 0-literal disambiguation in example test files)
that stay on the ladder branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wing paths

Bridge::executeVia and RemoteServer only ever schedule callbacks that
don't throw, so run()'s catch(std::exception&)/catch(...) arms around
entry.callback() had zero hit count. test_timeout_scheduler.cpp
exercises TimeoutScheduler directly: a callback throwing
std::exception, a callback throwing a non-std::exception, a throwing
callback not blocking a later callback from firing, cancel() on an
unknown handle, and cancel() before the deadline elapses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Yaraslaut
Yaraslaut force-pushed the framework/async-attach-and-execute-deadline branch from d0099e4 to 2475c77 Compare August 13, 2026 06:20
@Yaraslaut Yaraslaut added the no docs update Skip the header<->spec sync gate for this PR label Aug 13, 2026
Yaraslau Tamashevich and others added 4 commits August 13, 2026 09:44
…and destroyed-Bridge/binding paths

codecov flagged 20 uncovered lines in bridge.hpp's patch diff. Six of
those are a genuine tool-rendering artifact (the first statement after
a block containing a conditional early-return shows a 0 hit count
though the next line, executing exactly as often, shows the correct
count -- the same pattern already confirmed for PR #71's bridge.hpp
weak_ptr line). The rest are real: no test exercised a backend whose
attachModelAsync/registerModelSharedAsync throws synchronously, or an
async attach/bind reply arriving after the Bridge or its binding is
already gone.

Adds ThrowingDispatchBackend (throws from both dispatch entry points)
and four lifetime tests: attachHandlerAsync/ensureBoundAsync's
out-of-frame success callback is a no-op once the Bridge is gone, and
once only the binding is gone. The two Bridge-gone cases co-own the
backend via the existing AsyncBackendShim so completeNext() can still
run after the Bridge itself is destroyed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… inline-onError path

Two more of codecov's flagged lines were real gaps: ensureBound (the
synchronous counterpart to ensureBoundAsync) had no test calling it
twice on the same binding to exercise its already-bound early return,
and ensureBoundAsync's error callback's parkIfInFrame no-op was
untested even though attachHandlerAsync's identical branch already
was.

Also rewords the two 'binding is gone' test names/comments added in
the previous commit: reading BridgeHandler::execute's dispatch closely
shows the binding itself is pinned by the pending onDone closure
independent of the BridgeHandler, so destroying just the handler never
actually drops the binding's last strong reference through this path --
the tests still verify a real case (dropping the handler while an
attach is in flight must not crash), just not the specific 'binding is
gone' branch their original names claimed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-fire guard

Two more of codecov's flagged lines were real: execute()'s try/catch
around ActionKeyTraits<Action>::key(action) had no test whose key()
throws, and detail::parkIfInFrame's 'handoff.fired' branch (a backend
violating its documented one-callback-per-dispatch contract) had no
test backend that actually violates it.

Adds ARThrowingKeyTouch (a payload-keyed action whose ActionKeyTraits
specialization throws from key(), written directly rather than via
BRIDGE_KEY_FROM since that macro's generated body can't be made to
throw) and DoubleFiringBackend (calls attachModelAsync's onRegistered
twice inline, on purpose), plus one test each confirming
execute()/attachHandlerAsync still resolve exactly once through
onError/onDone rather than letting the second call through or
escaping the no-throw contract.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… is gone' branch

The earlier 'binding is gone' tests went through BridgeHandler::execute,
whose own dispatch closure captures the binding by value -- so the
binding never actually died while that dispatch was in flight (already
noted in their comments). Calling attachHandlerAsync/ensureBoundAsync
directly with an onDone that captures nothing binding-related removes
that hidden strong reference, so dropping the test's own shared_ptr
before completing the reply now genuinely exercises weakBinding.lock()
failing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Yaraslaut
Yaraslaut merged commit 1519be1 into master Aug 13, 2026
22 checks passed
Yaraslaut pushed a commit that referenced this pull request Aug 13, 2026
…dispositions

Filed one GitHub issue per genuinely open finding (26 total: 003, 005-009,
011-014, 017, 019-027, 029-032, 034, 036), each with a reproducer or a
desired-behavior/suggested-fix-direction section, cross-referenced back
into its finding .md file via a new `issue:` frontmatter line.

While drafting these, re-verified every finding's disposition against the
current tree instead of trusting stale frontmatter, and found five whose
`disposition: open`/`fix-scheduled` no longer matched reality -- the
underlying fix had already landed on this branch after the finding was
filed, but the frontmatter was never updated:

- 001 (async shared/keyed attach): registerModelSharedAsync/
  attachModelAsync exist and are wired through Bridge -- fixed.
- 002 (client execute deadline): Bridge::setExecuteDeadline exists,
  tested -- fixed.
- 004 (fault-injection wire proxy / deterministic strand interleaver):
  both exist under examples/common/testkit/ -- fixed.
- 028 (ladder tests inherit Lightweight's -Weverything warnings): fixed
  by 18d9438 and e737a77 (SYSTEM include demotion).
- 033 (BackendRig switch missing default): fixed by e737a77 (added the
  default: label).

No issue was filed for these five, nor for 010/015/016/018
(documented-limitation: intentional design tradeoffs, not bugs) or 035
(already tracked via PR #42's framework fix). 36 findings total = 26
issues + 10 correctly left without one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut pushed a commit that referenced this pull request Aug 13, 2026
Same fix as PR #42's identical bug, found the same way: the commit that
became invalid was correct on this branch's own prior state but not
once rebased onto current master, where the pendingCalls() counter
feature (merged via #70) shares this exact lambda and needs 'this' to
decrement it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no docs update Skip the header<->spec sync gate for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant