test: raise mesh-session.ts mutation score to 92.91% - #112
Merged
Conversation
…se/handshake edge cases Assert sendGossipUpdate rejects extensions colliding with the addresses and snapshot-seconds mandatory fields specifically, not just device. Assert the extension-key pattern rejects trailing garbage after a valid prefix and a valid suffix reached from a non-domain-qualified start, proving the regex is anchored at both ends rather than merely searched. Assert close() while connected finalizes state to closed/"closed by you" and actually invokes the underlying connection's close(). Add FakeConnection.isClosed and .endStream() so a clean remote hang-up can be distinguished from a caller-initiated close in tests. Assert a frame already queued at the moment close() runs is dropped rather than applied, and that a second handshake frame arriving after the first has already settled negotiation cannot re-negotiate. Assert a rejected handshake's reason is exactly "no shared domains or version", and that a negotiated handshake stays negotiated once its own timeout later elapses instead of flipping to unanswered.
Assert close() cancels an armed handshake timeout and a pending scheduled reconnect (via vi.getTimerCount()), rather than leaving either running after the session is torn down, and that a reconnect scheduled but not yet fired never dials again once closed. Assert close() called while a reconnect is pending still finalizes state to closed/"closed by you", the same as closing from any other state. Assert a reconnect attempt whose own dial rejects (as opposed to the resulting connection later failing) is treated as a genuine disconnect and reported through state.reason, rather than the rejection being silently dropped by the scheduler.
…equest backlog delivery Assert sendManageRequest assigns strictly increasing request-ids across successive calls, and refuses to send both before the first connect and again after a connection has failed and closed (not just before the first connect ever happens). Assert a request with no timeoutMs given never resolves via the internal timeout race, by advancing time and then delivering a real response and checking it still wins. Assert respond() emits a session event reflecting the sent response frame, and that a manage-request received before anything was iterating incomingManageRequests is still delivered once iteration starts, drawn from its own backlog rather than being lost. Add a yielded() helper to narrow an IteratorResult to its value without an unsafe cast, since AsyncIterator's default TReturn=any otherwise infers `any` for .value even after checking .done.
…aults Assert the events async iterator's next() result carries done: false both for a live event delivered after the wait began and for one already backlogged before anyone started iterating. Assert acceptMeshSession advertises no addresses when none are given (rather than falling through to some other default) and labels its connection state "accepted" when no label option is given. Strengthen the "refuses connect()" assertion to check the actual rejection message reachable through the public API, rather than any throw.
…d revocation backlog Assert both methods emit a session event reflecting the sent frame, and refuse to run both before the first connect and again after a connection has failed and closed. Strengthen the revocation-announce receive test to check done: false on each yielded entry, and add delivery from the backlog for an entry queued before anyone was iterating revocationAnnouncements.
Assert that when a dial resolves after the caller has already closed the session, the resulting connection is closed immediately rather than wired up (no handshake or self-advert ever sent), matching the intent that close() cancels any connection attempt still in flight.
…e()-rescued one The respond()/sendRevocationAnnounce/sendGossipUpdate "emits a session event" tests were awaiting a second event via nthEvent, then always calling session.close() afterward. Since frameLog.push() happens unconditionally before each method's own emit() call, and close() unconditionally emits one final event of its own, a missing emit() in any of the three methods was silently masked: the awaited event just resolved later, from close()'s own trailing emit(), with a frameLog snapshot that still looked correct by coincidence. Add a withinShortWait() helper that races the next event against a short real timer, so the assertion only passes if the event was already available immediately after the send call, before close() is ever invoked. Apply it to all three "emits a session event" tests and reuse it in place of the ad hoc race already inlined in the close()-drops-a-queued-frame test. Also add a final-state assertion to the "closes a dial that only completes after close()" test, covering the state.status === "connecting" branch of close()'s own three-way state check -- the existing assertions only checked the late connection's own side effects, not that the session's state actually settles to closed.
Mearman
marked this pull request as ready for review
September 13, 2026 15:32
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
resolveDial, a plain let reassigned only inside a Promise executor nested in an object-property arrow function, lost its non-null narrowing across several intervening awaits, typing the eventual call site as never -- caught by tsconfig.node.json's own dedicated typecheck of test/, which the default tsconfig.json doesn't cover. Wrapping the resolver in an object property instead of a bare closed-over variable avoids the narrowing loss entirely, matching the resolvePeerDeviceId pattern used elsewhere in this same file.
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
1 participant
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.
Fixes #68
wire-mesh-core#67's baseline scored tokens.ts and revocation-view.ts at 80.00% and 100.00% already (nothing to do there). mesh-session.ts scored 67.32% (145 killed, 26 timeout, 83 survived); this raises it to 92.91% (204 killed, 32 timeout, 18 survived out of 254 scoreable mutants) by tracing every survivor individually and adding a real assertion where the gap was genuine, or confirming and documenting equivalence where it wasn't.
Along the way this caught a real bug in the test suite itself: three "emits a session event" tests awaited a second event, then unconditionally called session.close() afterward. Since close() always emits one final event of its own, a missing emit() in respond()/sendRevocationAnnounce/sendGossipUpdate was silently masked -- the awaited event just resolved later from close()'s trailing emit(), with a frameLog snapshot that still looked correct by coincidence. Fixed with a withinShortWait() helper that races the next event against a short real timer, so the assertion only passes if the event was genuinely available immediately, before close() is ever invoked. Verified by manually re-applying each exact mutation and confirming the fixed tests now fail against it.
Verification: fresh tsc --noEmit, eslint, and pnpm test (13 files, 237 tests, including 75 in mesh-session.test.ts alone) all pass.
The 18 remaining survivors
Each traced individually against the real call graph, not assumed:
if (feedCancelled) return) already no-ops the only reachable case where this outer check would matter.connection===null || state.status!=="connected"(three call sites: transmit's own guard, sendRevocationAnnounce, sendGossipUpdate): logically redundant given the codebase's own invariant that state.status==="connected" is only ever true after connection is set non-null in the same synchronous block -- connection===null always implies the right operand is already true too.