test(core): assert RoomLifecycle's CRUD, join/leave, and grant logic directly - #129
Merged
Merged
Conversation
…directly RoomLifecycle was previously exercised only indirectly through end-to-end integration tests. This adds direct, DI-based unit tests covering every public method: createRoom (slugging, secret-room naming, duplicate detection, root-grant minting), getRoom, listRooms (secret-room visibility), joinRoom/joinRemoteRoom (local-vs-remote gating, invitation consumption, subscribedRooms/room_members/federation notification), leaveRoom/leaveRemoteRoom (local-vs-remote gating, cascading destroy of an empty owned room), refreshRoomMembers, requestDmAccess, inviteToRoom/declineInvite, revokeMemberGrant, kickFromRoom, and destroyRoom. Uses real generated identities and real minted capability tokens throughout, not opaque placeholders: this class genuinely mints tokens and parses roomJoinOkSchema/roomMembersOkSchema's own structural COSE shape, which an arbitrary placeholder string fails outright. Split across two files to stay under the repo's max-lines cap.
A full mutation run against the current test suite surfaced 26 survivors against a 65.20% baseline; this closes the real gaps among them (mutation score up to 89.76% before this commit, further improved by these fixes): - Every thrown CommsError's own message, not just its code, is now asserted where a test previously checked only the code -- several StringLiteral mutants inside error-message template strings otherwise survive untouched since no assertion ever reads the message text. - joinRemoteRoom and refreshRoomMembers now assert the built room's own memberJoins and federated fields, not just members/name/type -- both were constructed but never independently verified. - refreshRoomMembers now asserts the exact scope passed to sendRoomRequest, and that an existing room's federated flag survives a refresh while a fresh one defaults to false. - joinRoom's own agent-subscription branch now asserts the bumped version, not just the mutated array. - leaveRoom's subscribedRooms filter is now tested against an agent subscribed to more than one room, since a single-entry array can't distinguish "filter out this room" from "filter out everything". - leaveRoom's and kickFromRoom's federated-notification branches are now tested on both sides (federated and not), not just the positive case. - leaveRemoteRoom's reason-omission (undefined, not merely absent-valued) is now asserted via leaveRoom's own remote-leave path, which never passes one. - kickFromRoom now asserts the room's version bump and that the target is actually gone from the recomputed members list, not just that the underlying recordMemberOp calls were made. The remaining survivors are documented in place rather than chased: four are the same Map/rooms.set-on-an-already-stored-reference no-op this repo's own mutation-testing work has already documented elsewhere (agent-registry.ts, delivery-engine.ts), and three are error branches genuinely unreachable against this suite's real, validly-generated crypto identities. Split across three files (from two) to stay under the repo's max-lines cap after these additions.
A fresh mutation run (94.49%, up from 89.76%) surfaced four further real gaps: refreshRoomMembers's invited-field fallback with no existing room was never asserted; inviteToRoom's own room.version bump, and the exact scope it sends its outbound request under, were both unobserved. Also documents two further equivalent mutants following the same same-object-reference Map.set pattern already documented elsewhere in this file: joinRoom's and leaveRoom's own agents.set(agentId, agent) calls, alongside their already-documented rooms.set(roomId, room) ones. Directly verified (by manually mutating the source locally and confirming the test suite genuinely fails) that leaveRoom's own destroy-trigger condition is correctly covered by the existing "does not destroy" test despite Stryker's own report listing it as a survivor -- not chased further, since the coverage is real. Split declineInvite and revokeMemberGrant out to room-lifecycle-remote.test.ts to stay under the repo's max-lines cap after these additions.
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. |
|
🎉 This PR is included in version 2.21.19 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Summary
Closes #109. Part of the mutation-testing epic tracked in #64.
Mutation score: 65.20% -> 94.88% (241/254 scoreable, verified with a full local Stryker run on a dedicated machine). The thirteen remaining survivors are almost entirely provable equivalent mutants (same-object-reference Map.set no-ops, documented in place) or error branches genuinely unreachable against real, validly-generated crypto identities.
Test plan
pnpm typecheckpnpm lintpnpm test(662 passed)