Skip to content

ci: run both leak harnesses nightly and gate on injected leaks - #254

Merged
truthixify merged 3 commits into
wraith-protocol:developfrom
collinsezedike:test/scheduled-leak-regression
Oct 17, 2026
Merged

truthixify merged 3 commits into
wraith-protocol:developfrom
collinsezedike:test/scheduled-leak-regression

Conversation

@collinsezedike

Copy link
Copy Markdown
Contributor

Summary

test:leak (the streaming scan slope harness) existed but no workflow ran it, so memory regressions in the streaming path could accumulate unnoticed. The constructor-count heap snapshot test was scheduled, but the streaming path has different allocation behaviour that it does not cover. This schedules both harnesses, uploads diagnostics only on failure, and adds a job that proves a deliberate leak actually trips each gate.

Changes

  • stream-leak-regression: nightly job that runs pnpm test:leak on Node 22 and uploads scan-leak-diagnostics.json when the gate fails.
  • heap-regression: same harness as before, but diagnostics now upload only on failure and are limited to the compact heap-diff.json.
  • leak-injection: nightly job that runs each harness with a deliberately retained leak (SCAN_LEAK_INJECT=1, HEAP_LEAK_INJECT=1) and fails when either gate reports success, proving the thresholds catch real leaks.
  • scan-leak.test.ts: writes a compact scan-leak-diagnostics.json (samples, fitted slope, threshold) and only writes the large heap-final.heapsnapshot when the gate trips. Adds SCAN_LEAK_ANNOUNCEMENTS, SCAN_LEAK_ITERATIONS, SCAN_LEAK_SAMPLE_INTERVAL, SCAN_LEAK_THRESHOLD_MB, and SCAN_LEAK_INJECT knobs with CI-feasible defaults, and requires --expose-gc for a stable slope.
  • test:leak now launches Vitest through Node with --expose-gc --pool=threads, matching test:heap-leak, so forced GC keeps the slope threshold stable.

Notes

The streaming harness defaults were reduced because the previous 10000 iterations over 10000 announcements never completed within a CI budget. The threshold stays at 0.2 MB per sample step, and the reduced workload keeps enough samples for a stable least-squares fit. The full heap snapshot is written on failure for triage but is not uploaded, keeping the nightly artifacts compact.

Closes #246

@drips-wave

drips-wave Bot commented Oct 16, 2026

Copy link
Copy Markdown

@collinsezedike Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@truthixify truthixify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deliberate leak proof does not trip the streaming gate. Reproduction: SCAN_LEAK_ANNOUNCEMENTS=1 SCAN_LEAK_ITERATIONS=300 SCAN_LEAK_SAMPLE_INTERVAL=25 SCAN_LEAK_INJECT=1 pnpm test:leak exits 0 with a 0.0149 MB-per-step slope. LeakSentinel retains an 8 KB Uint8Array, whose backing allocation is largely external to V8 heapUsed; additionally, 25 injected buffers nominally add about 0.195 MB per sample, below the 0.2 MB threshold even if fully counted. The nightly leak-injection job will therefore flag the detector as ineffective. Please inject heap-accounted retention or measure external and array-buffer memory too, set the injected growth clearly above threshold, and keep a regression proving the injection command exits nonzero.

@collinsezedike

Copy link
Copy Markdown
Contributor Author

@truthixify thank you for the detailed root cause analysis. You're right. The 8 KB Uint8Array backing is external to V8 heapUsed, and even fully counted, 25 buffers at 0.195 MB/step fell below the 0.2 MB threshold. I've addressed both issues:

  • Memory accounting (memoryMB) now sums heapUsed + external + arrayBuffers instead of just heapUsed, so Uint8Array backing memory is counted.
  • Injection magnitude increased LeakSentinel payload from 8 KB to 64 KB. At the CI-injected settings (200 iterations, sample interval 20), this produces 10 samples each retaining 64 KB, yielding a slope well above 0.2 MB/step.
  • Workload sizing reduced the default SCAN_LEAK_ANNOUNCEMENTS from 2000 to 50 so the test completes within CI budget while still exercising scanAnnouncements.

Right now, when,

  • SCAN_LEAK_INJECT=0, slope ≈ 0.002 MB/step and test passes
  • SCAN_LEAK_INJECT=1, slope ≈ 1.96 MB/step and test fails

The leak-injection job should now trip correctly.

@truthixify truthixify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated memory metric detects the injected retained ArrayBuffer payload while the normal harness remains below threshold. Approved.

@truthixify
truthixify merged commit 053412a into wraith-protocol:develop Oct 17, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Wave 10] Run the streaming scan leak test in scheduled CI

2 participants