[kahrendt]

Summary

No listener, role or provider callback may call start() or stop(), the same rule #259 set for loop() and enforced the same way (documentation, no runtime check). With all three off-limits, no drain ever runs beneath another and no role's main-loop teardown half runs while one of its callbacks is on the stack. That removes the only reason for the re-entrancy guards in the drain, so they go. A consumer that wants to stop or restart in response to a callback records the request and acts on it between two loop() calls, which is what the ESPHome hub already does: it calls stop()/start() only from its own loop() and on_shutdown().

Change

Removed, each of which only served a callback that calls stop() (and maybe start()) beneath the drain that fired it:

• EventState::drain_generation, its bump in stop(), and every check in drain_inbox(): the event-ring skip and per-event abort, the pairing-note checks, the gates before each role drain and the group drain, and the check before the deprecated on_group_update().
• The pairing notes' move-out-and-back plus the reserve() a nested drain needed. The drain uses its buffer in place. The internals Memory row for the extra buffer goes too.
• start()'s STOPPING branch and its warning, which only a callback inside stop() could reach. The STOPPING state stays: other threads, and connect_to()/disconnect() from callbacks inside stop(), still need is_started() to read false.
• The accepts(generation) checks after player, artwork and controller callbacks and at the head of their drains and drain_scheduled_state(), then RoleTeardown::accepts() itself.
  • Between a drain's catch-up and those head checks only the clear callback runs, so they only caught a callback calling stop().
  • A protocol-task teardown in that window now gives "state, then clear on the next drain". That is the outcome it already gave when it landed just after the check.
• The player's indexed loop, teardown_reentered and the erase clamp. The awaiting events change only from the ring drain and the catch-up, neither of which can run inside the loop.
• Metadata's TeardownTracker::completed() check before the deprecated on_metadata(), and completed() itself.

The protocol task's teardowns stay caught by the generation stamps (keep_current_payload(), event_is_current(), ScheduledStateSlot's sequence check), which are unchanged.

One take did depend on the removed checks: the player's STREAM_START took whatever stream parameters were pending and dropped them on a stamp mismatch. A teardown during an earlier callback in the same drain, plus the next connection's stream/start, left the next START reading the previous stream's parameters. The new GenerationSlot::take_stamped() takes a payload only under the given stamp and leaves a later one pending. The player takes its parameters under the START's stamp, and the visualizer its config the same way.

Docs: client.h (start(), stop(), LifecycleState), integration guide (Stopping and Restarting, Thread Safety Summary, string_view section), conventions.md, the house-patterns checklist, and internals.md (the re-entrant section becomes "Callbacks During Dispatch", plus stop() steps 0 and 6).

Also removed, from what #261 added for pairing: the skip before each deprecated pairing callback and the nested-drain comments on its text buffer.

Tests

• Deleted StopFromInsideADrainCallbackIsSafe, StopFromInsideAPairingCallbackDismissesThePromptOnce and StopFromTheStoppedCallbackFindsTheHoldReleased.
• Dropped the stop/start rows from MetadataSlot, the "stop() from ..." rows from the metadata and group listener-form tables, and the "stop() from the first view form" row from PairingCallbacksReachBothForms.
• CallbackDuringStopCannotRecurse becomes CallbackDuringStopSeesTheStoppedClient: it keeps the stopped-state reads and now checks the request slot, so a connect_to() or disconnect() the callback posted fails it (mutant: drop connect_to()'s running check).
• New TeardownReorder.AStreamStartTakesOnlyItsOwnStreamParams: a teardown plus the next stream's stream/start during on_volume_changed(), with a control row. A mutant restoring the unconditional take makes the next stream's on_stream_start() read no sample rate.
• The metadata-form row where the protocol task tears down during on_track_metadata() stays as the control: the deprecated form still follows.
• ASan, TSan and the allocation tests run clean, and an ESP-IDF 5.5 esp32s3 build of tests/esp_idf compiles the library without warnings.

[Copilot]

🟡 Changes recommended

A critical race allows an old START to discard a newer stream’s parameters after protocol-task teardown, causing the next stream-start callback to read stale parameters.

3 open issues

• Old START can consume parameters from a newer stream · New
• Clarify when role drains catch up after teardown · New
• Test that connect and disconnect post no requests after stop · New

What changed in this PR

Extends #259’s callback contract to forbid calling start() or stop() from callbacks. Consumers must defer stop or restart requests until between loop() calls, allowing the client and role drains to remove nested-lifecycle handling.

Changes:

• Documents the callback restrictions and deferred lifecycle-request pattern.
• Removes re-entrancy guards and nested-drain buffer handling.
• Revises lifecycle tests and documentation to match the new contract.

File	Description
tests/​test_source_role.cpp	Removes callback-triggered stop coverage.
tests/​test_encrypted_lifecycle.cpp	Removes stop-from-callback listener cases.
tests/​test_client_lifecycle.cpp	Revises stopped-state checks and removes re-entrant lifecycle cases.
tests/​README.md	Updates the lifecycle test summary.
tests/​CMakeLists.txt	Updates white-box test access explanations.
src/​teardown_tracker.h	Removes callback re-entry checks and unused accessors.
src/​source_role.cpp	Removes obsolete re-entrant teardown comments.
src/​scheduled_state_slot.h	Removes the callback re-entry guard.
src/​player_role.cpp	Simplifies stream-event iteration and removes generation guards.
src/​metadata_role.cpp	Removes the teardown check between listener forms.
src/​metadata_role_impl.h	Updates callback-dispatch documentation.
src/​controller_role.cpp	Removes the drain’s additional generation check.
src/​client.cpp	Removes nested-drain guards and simplifies pairing dispatch.
src/​artwork_role.cpp	Removes callback-related generation guards.
include/​sendspin/​client.h	Documents lifecycle callback restrictions.
docs/​internals.md	Revises dispatch, shutdown, and allocation descriptions.
docs/​integration-guide.md	Documents deferred stop/restart requests and callback delivery.
docs/​conventions.md	Replaces re-entrant teardown requirements with callback restrictions.
.claude/​skills/​house-patterns/​SKILL.md	Aligns the review checklist with the new contract.

_{Review effort: Max}