test(core): raise StaleAgentChecker mutation score to 95.65% - #118
Conversation
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. |
No test file existed for this module at all -- its 45.65% mutation score came entirely from whatever mesh-store.ts's own integration tests happened to exercise indirectly. Covers both responsibilities directly against injected StaleAgentCheckerDeps: dead-pid detection (marks an active agent with an unresponsive pid offline, notifies, broadcasts, drops its peer info; leaves a live one untouched; never re-probes an already-offline agent) and long-offline purging (removes an agent past the purge threshold from every map; leaves a recent one alone; never purges a non-offline agent regardless of age), plus start()/stop() idempotency and timer lifecycle.
…hecker broadcastPatch call counts can't distinguish a leaked second timer from a single one: probeStaleAgents mutates an agent's status to "offline" synchronously, before its own first await, so a second un-guarded interval firing in the same tick finds nothing left to mark dead even when a real second setInterval exists underneath. Asserting directly on setInterval/clearInterval's own call counts is what start()/stop()'s guards actually exist to control, so that's what these tests check instead. Also adds the exact-boundary purge case (an offline agent whose startedAt lands precisely on the purge threshold, not just just inside/outside it) and size assertions on every purged map, closing the gap between "the intended entry is gone" and "nothing else got touched." Raises this file's own mutation score from 80.43% to 95.65% -- the two remaining survivors are genuine equivalents (re-set()-ing a Map to the same object reference it already holds; Map.delete() on a key that was never present), documented in the test file itself rather than chased with a contrived assertion.
The @exadev/eslint-config adoption that landed on main since this branch was created tightened magic-number checking; named the threshold-derivation and test-multiplier literals this file's own earlier commits introduced.
91e005f to
a1bf156
Compare
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
🎉 This PR is included in version 2.21.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #112.
Adds dedicated test coverage for StaleAgentChecker (no test file existed for it before this) covering dead-pid detection, long-offline purging, and start()/stop() timer lifecycle. Raises the mutation score from 45.65% baseline to 95.65% (well above the repo's 80% target). The two remaining survivors are genuine equivalent mutants (documented in the test file itself), not real gaps.
Along the way, found and fixed a real, unrelated bug:
web-server.integration.test.tsnever passed an isolatedcoordinatorPorttocreateWebServer, so its own MeshStore defaulted to the well-known port 19876 -- the same port a real, already-running agent-comms bridge on this machine binds as coordinator. Every test in that file silently joined the live, unrelated process instead of standing up its own isolated mesh, then hung waiting on a handshake the real bridge was never going to answer correctly, until Vitest's 30s per-test timeout killed it (confirmed reproducible standalone, not a Stryker-sandbox artifact). Every other coordinator-using integration test in this repo already picks its own distinct port for exactly this reason; this file was the one exception.