fix(backend): stop mock modem re-broadcast clobbering merged client state (fixes flaky shard-2 E2E)#155
Merged
Merged
Conversation
The mock feedback tick (runMockFeedbackBroadcast, ~5s) re-broadcast FULL
modem state for every modem via broadcastModems(undefined). Since the
frontend now merges each status.modems frame field-by-field
(subscriptions mergeModemList) rather than wholesale-replacing, and
buildModemMessage always includes available_networks (and config) in a
full descriptor, that periodic full re-send overwrote client-side modem
fields with the backend's empty defaults.
This deterministically broke the desktop shard-2 E2E job. A pending modem
scan in modem-config-surface.spec.ts injects available_networks over the
page socket (dev.emit is page-scoped and never persisted in modemsState);
when a feedback tick then re-sent available_networks={} it flipped the
scan signature and false-confirmed the pending scan, re-enabling the scan
button (expect(scan).toBeDisabled() -> enabled). The race only lands
inside the assertion window on slower CI runners, so it passed locally
but failed byte-identically in CI.
Align mock mode with the real device: broadcastFromDiff now sends a full
descriptor only for newly-added modems and a status-only partial for
every changed modem (the non-mock branch already did exactly this). A
periodic status refresh no longer re-sends available_networks/config, so
it can never clobber merged client state. The initial full snapshot still
reaches clients via the post-login buildModemsMessage() push and the
discovery diff's added entries; real-device behavior is unchanged.
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
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
In mock mode,
broadcastFromDiff(apps/backend/src/modules/modems/modem-update-loop.ts) re-broadcast the full modem descriptor for every modem on every reconcile diff (broadcastModems(undefined)). This change makes mock mode use the same path as a real device: a full descriptor only for newly-addedmodems, a status-only partial for everychangedmodem. One file, +13/-9.Why
The desktop shard-2 E2E job (
modem-config-surface.spec.ts:100 — "a modem scan holds pending across same-list re-broadcasts…") failed byte-identically on every PR:Root cause (a timing race, not a cross-file leak):
available_networksover the page socket viadev.emit, which is page-scoped and never persisted in the backendmodemsState— so the backend's realavailable_networksfor that modem stays empty ({}).runMockFeedbackBroadcast, ~5s) re-sent the full modem state on every signal fluctuation, andbuildModemMessagealways includesavailable_networks(andconfig) in a full descriptor.status.modemsframe field-by-field (subscriptions.svelte.tsmergeModemList, shallow), so an incomingavailable_networks:{}overwrote the injected set.The race only lands inside the 5s assertion window on slower/contended CI runners (backend up for minutes across ~25 prior shard-2 specs), so it passed locally but failed deterministically in CI.
sim_lockis only serialized when present, which is why the siblingsim-unlock-surface.spec.tssurvives the same tick;available_networks/configare always serialized, so they get clobbered.A real device already sends status-only for a
changedmodem (the!shouldUseMocks()branch), so it never clobbers the scan list. The mock's full re-send was a stale artifact from before the frontend merged per-field. This aligns the two; real-device behavior is unchanged.How to verify
MOCK_FEEDBACK_BROADCAST_INTERVAL_MS=250and runmodem-config-surface.spec.ts— it fails at thetoBeDisabledassertion (byte-identical to CI). With this fix,-g "holds pending" --repeat-each=8→ 8/8 pass.bun test:mock-feedback-broadcast,modem-available-networks-contract,modems-state-cache,modems-configure-applied,mock-modems-output,mock-monitor,monitor-manager,modem-migration, sim-state suites → 75 pass, 0 fail.--project=desktop --workers=4(grep-invert@visual|@a11y|@gallery): full suite → 226 pass / 3 skipped / 0 fail; modem/network/sim blast-radius subset → 25 pass.bun tsc --noEmitexit 0;biome checkclean.Risks
buildModemsMessage()push and the discovery diff'saddedentries. The mock-feedback unit test asserts only.status.signal/.status.connection+ dedupe counts, all preserved by a status-only partial.