Skip to content

WIP: more fuzz tests#4962

Merged
NathanFlurry merged 1 commit intomainfrom
05-04-wip_more_fuzz_tests
May 5, 2026
Merged

WIP: more fuzz tests#4962
NathanFlurry merged 1 commit intomainfrom
05-04-wip_more_fuzz_tests

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

This was referenced May 4, 2026
Copy link
Copy Markdown
Member Author

NathanFlurry commented May 4, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

PR #4962 Review: WIP: more fuzz tests

Overview

This PR adds two new fuzz-test facilities:

  1. Sleep-close fuzz - a standalone script + actor to stress-test the force-sleep to non-hibernatable WebSocket close path
  2. Abort listener vars test - a fixture actor + driver test reproducing a user-reported crash where c.vars is undefined inside an abort listener captured in onWake, caused by a race between the abort TSF and cleanupNativeSleepRuntimeState

The PR also updates MockAgenticLoopPanel to support queued (concurrent) inferences, matching a server-side change to mock-agentic-loop.ts.


Issues

tests/driver/actor-sleep.test.ts

  • Line 1119: bare setTimeout(resolve, 1500) - This timing-based wait lets the actor sleep and re-wake between cycles. Per project conventions (Never paper over flakes with retry loops or bumped waits), this should be replaced with an event-driven approach or at minimum carry a mandatory inline comment explaining why polling is unavoidable here. As written it will silently become flaky under load.

  • Line 1135: skips the last abortVarsSeenPerWake element (length - 1) - No comment explains why. A reader cannot know whether the last slot is excluded because it represents an incomplete wake cycle, a timing artifact, or something else. Add a comment.

examples/kitchen-sink/frontend/App.tsx

  • handleStarted: validation errors do not abort execution - If nextRequest.requestId !== message.requestId or if activeRequestRef.current is already set, markValidationError is called but execution continues. The local queues and the active-request slot are then mutated even after detecting corruption, making subsequent observations unreliable. Return early after each markValidationError call.

  • verifyAll: expectedState snapshot may be stale - activeRequestRef.current is captured at the top of verifyAll, but the snapshot is surfaced via setLastVerification only after the async handle.verifyAll call returns. If an inference completes during that await, the displayed active field will not match the actor final state. Minor observability issue, worth a comment.

examples/kitchen-sink/scripts/sleep-close-fuzz.ts

  • Line 608: empty Authorization header - When TOKEN is empty, this sends Authorization: "" which is semantically different from omitting the header. Use a conditional spread.

  • Line 635: leakTimeout timer is never cancelled when closePromise wins - Promise.race resolves immediately but the underlying setTimeout still fires after LEAK_THRESHOLD_MS. Across many parallel workers this accumulates orphaned timers. Pass an AbortSignal or clear the timer on the winning path.

  • Lines 547-551: createClient instantiated per iteration - If the client holds persistent connections or module-level state this is O(iterations x workers) overhead. If construction is cheap, fine; if not, hoist outside the iteration loop.

examples/kitchen-sink/src/actors/testing/sleep-close-fuzz.ts

  • Line 866: magic number for WebSocket.OPEN - websocket.readyState !== 1 is fragile. Use WebSocket.OPEN or define a named constant.

Minor / Style

  • sleep.ts fixture: the cast to { isStopping: boolean } | undefined on a value already typed by createVars is unnecessary unless the runtime can return undefined outside the type system. If defensive, add a comment.

  • sleep-close-fuzz.ts line 678: if (workerIndex * STAGGER_MS > 0) is clearer as if (workerIndex > 0) since STAGGER_MS is always positive.


What is Good

  • The fuzz script is well-structured: meaningful stats (p50/p95/p99 close latency), configurable via env vars, clear LEAK vs error vs ok distinction, hard exit code on leaks.
  • The deliberate micro-task drain in onSleep to make the abort-TSF race observable is a clever technique.
  • The test comment on the abort listener race is thorough and includes relevant file/line references - exactly the right level of documentation for a subtle concurrency bug.
  • Queued inference in mock-agentic-loop.ts is cleaner than the previous reject-if-busy approach; the activeInference === inference guard correctly handles the concurrent-completion case.

Summary

The core ideas are solid. The main blockers before merging are the timing-based setTimeout(1500) in the driver test (flake risk) and the missing early returns in handleStarted (correctness risk). The rest are polish items.

@NathanFlurry NathanFlurry force-pushed the 05-04-wip_more_fuzz_tests branch from c7dee23 to e590e33 Compare May 5, 2026 12:09
@NathanFlurry NathanFlurry force-pushed the rivetkit-db/onmigrate-savepoint branch from c4c18dc to 627fd20 Compare May 5, 2026 12:09
Base automatically changed from rivetkit-db/onmigrate-savepoint to main May 5, 2026 14:58
@NathanFlurry NathanFlurry merged commit e590e33 into main May 5, 2026
10 of 24 checks passed
@NathanFlurry NathanFlurry deleted the 05-04-wip_more_fuzz_tests branch May 5, 2026 14:58
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.

1 participant