chore(rivetkit): pass request id from driver to conn so we can correlate underlying requests with connections

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 4, 2025 8:26pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 4, 2025 8:26pm
rivet-inspector	Ignored	Preview		Nov 4, 2025 8:26pm
rivet-site	Ignored	Preview		Nov 4, 2025 8:26pm

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• ci: trigger rust ci for generated bare artifacts #3346
• chore(rivetkit): debounce ws message ack #3345
• chore(rivetkit): auto-update auto-start engine version #3344
• chore(guard): handle cors on the gateway #3341
• chore(guard): add support for routing runner ws to /runners/connect #3340
• chore(guard): simplify gateway routing #3339
• fix(guard): include method in cache key #3338
• chore(engine): move smoke test to engine/tests/ #3337
• chore(rivetkit): add debug logging around sleep condition #3336
• fix(rivetkit): fix conns not being cleaned up if actor.createConn errors #3335
• fix(rivetkit): fix not pino bindings #3334
• chore(rivetkit): fmt #3331
• chore(rivetkit): correctly stringify engine runner request ids for logs #3330
• fix(rivetkit): ack ws messages #3329
• chore(runner): decrease timeout to 15s #3328
• chore(pegboard): log explicit gc reason for pending requests #3327
• fix(pegboard): fix tunnel ack from gateway #3326
• fix(rivetkit): explicitly handle errors in _onStop #3325
• chore(rivetkit): document sleep sequence better #3324
• fix(runner): close active requests after onActorStop has finished running #3323
• fix(runner): fix http requests not returning 503 when actor removed #3322
• fix(runner): dupe onActorStop call #3321
• chore: add rivetkit test writer agent #3320
• fix(rivetkit): prevent sleeping if there are any action hono requests, fixes inspector requests not resetiting sleep timer #3319
• chore(rivetkit): pass request id from driver to conn so we can correlate underlying requests with connections #3318 👈 (View in Graphite)
• chore(rivetkit): improve conn inspector properties #3317
• fix(rivetkit): don't throw error if websocket is not open, causing realtime to break after a connection disconnects #3316
• chore(smoke-test): add simple http test #3315
• chore(scripts): tee run logs to file #3314
• chore(engine): add db nuke subcommand #3313
• fix(rivetkit): fix "Method get ArrayBuffer.prototype.byteLength called on incompatible receiver #<ArrayBuffer>" #3312
• chore(scripts): add engine shell command #3311
• chore: set minimum node version to 20 #3310
• fix(fe): show connect page when there are no runner configs but runner names #3298 : 1 other dependent PR (#3306 )
• chore(rivetkit): log version with error logs #3303
• chore(rivetkit): log close code & reason in disconnect warning #3302
• feat(rivetkit): integrate websocket hibernation #3301
• fix: use correct vbare functions for runner tunnel #3300 : 1 other dependent PR (#3332 )
• feat(grafana): fix api dashboards #3193
• fix: docker compose build #3273
• chore: fix remaining warnings for all targets #3305
• fix(gas): fix from_workflow when creating standalone ctx from op ctx #3342
• ci: udpate ci filter to succeed if no relevant changes #3343
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

[claude]

Pull Request Review: Pass Request ID from Driver to Connection

Summary

This PR implements request ID tracking throughout the connection lifecycle, enabling correlation between underlying requests and connections for improved observability and debugging. The changes span TypeScript SDK code, RivetKit packages, and Grafana dashboard configurations.

Code Quality & Best Practices

✅ Strengths

1. Consistent Naming Convention: The rename from socketId to requestId better reflects the actual purpose - tracking requests rather than just sockets.

2. Type Safety: The PR maintains TypeScript type safety throughout by properly threading the requestId parameter through function signatures.

3. Good Separation of Concerns: The changes cleanly separate request tracking from connection management.

4. Comprehensive Coverage: The PR consistently updates all connection types (WebSocket, SSE, HTTP actions).

🔍 Areas for Improvement

1. Missing Type Definitions (rivetkit-typescript/packages/rivetkit/src/utils.ts:273-276)

   • The new bufferToString function is added but appears to already exist in the file. Please verify there's no duplication.
   • Consider adding JSDoc comments for the new utility function if it's genuinely new.

2. Potential Race Condition (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:880-895)

   if (
       requestId &&
       conn.__socket &&
       requestId !== conn.__socket.requestId
   ) {

   • While the stale disconnect check is good, consider if there's a race where conn.__socket could be null between the check and access.
   • Current code appears safe but could benefit from extracting conn.__socket to a variable for clarity.

3. Inconsistent Commenting (engine/sdks/typescript/runner/src/tunnel.ts:453)

   • Parameter type changed but function documentation may need updating to reflect the semantic meaning of requestId vs the old buffer-based ID.

4. Example Code Update (examples/kitchen-sink/src/backend/actors/demo.ts:160)

   sleepTimeout: 10_000,  // Changed from 2000

   • This appears to be an unrelated configuration change. Should this be in a separate commit/PR, or is it intentionally related to request ID tracking?
   • No explanation in the commit message for why sleep timeout was increased 5x.

Potential Bugs

⚠️ Medium Priority

1. Request ID Generation Timing (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:254)

   const requestId = generateConnRequestId();
   const conn = await this.createConn(
       {
           requestId: requestId,

   • The requestId is generated locally for executeAction calls, which is correct. However, ensure this doesn't conflict with externally-provided request IDs from the driver.
   • Consider adding validation to ensure request IDs are unique.

2. Missing Request ID in Logs

   • While the PR adds requestId to connection debug info (line 235-238 in instance.ts), verify that all relevant log statements now include the request ID for full correlation.
   • Some error paths might not include the request ID in their log context.

Performance Considerations

✅ Positive Impact

1. UUID Generation: Using crypto.randomUUID() for request ID generation is efficient and cryptographically secure.

2. No Additional Storage: Request ID is stored in the existing ConnSocket structure, no new data structures added.

💡 Suggestions

1. Grafana JSON Formatting (All dashboard files)
   • The formatting changes (multi-line to single-line arrays) reduce file size slightly, which is good.
   • However, these are auto-generated files from docker/template/. Ensure the template was updated as noted in CLAUDE.md.
   • Consider verifying the template regeneration was run: (cd docker/template && pnpm start)

Security Concerns

✅ No Significant Issues Detected

1. UUID Generation: Using standard crypto.randomUUID() is appropriate and secure.

2. Request ID Exposure: The request ID is logged for debugging but contains no sensitive information.

💡 Minor Consideration

1. Consider whether request IDs should be included in error messages sent to clients, or kept internal-only for security through obscurity.

Test Coverage

⚠️ Concerns

1. No Test Changes: The PR doesn't include any test updates or new tests for request ID tracking.

   • Consider adding tests to verify:
     • Request IDs are properly propagated through the connection lifecycle
     • Stale disconnect events are correctly filtered (instance.ts:880-895)
     • Request IDs appear in logs as expected
     • Request ID correlation works across reconnections

2. Integration Testing:

   • The kitchen-sink example timeout change suggests manual testing was done.
   • Consider documenting what manual testing was performed.

Additional Observations

Documentation

• The PR description is empty. Adding context about why request ID correlation is needed would be helpful for future reference.
• Consider adding inline comments explaining the stale disconnect check logic (instance.ts:880-895).

Driver Interface Changes

The fetch and websocket callbacks in RunnerConfig now require a requestId parameter:

fetch: (runner: Runner, actorId: string, requestId: protocol.RequestId, request: Request) => Promise<Response>;
websocket?: (runner: Runner, actorId: string, ws: any, requestId: protocol.RequestId, request: Request) => Promise<void>;

Breaking Change Alert: This is a breaking change to the driver interface. Ensure:

• All driver implementations are updated (appears complete in this PR)
• Version bump is planned if this is a public API
• Migration guide exists for external driver developers (if applicable)

Connection State Debugging Enhancement

The addition of requestId and driver to connection debug info (instance.ts:235-238) is excellent for observability:

requestId: conn.__socket?.requestId,
driver: conn.__driverState
    ? getConnDriverKindFromState(conn.__driverState)
    : undefined,

Recommendations

High Priority

1. ✅ Add tests for request ID propagation and stale disconnect handling
2. ✅ Verify Grafana dashboard template was regenerated properly
3. ✅ Add PR description explaining the motivation for request ID tracking
4. ✅ Document or separate the sleepTimeout change in the example

Medium Priority

1. Add JSDoc comments for the new request ID flow
2. Ensure all log statements include request ID where relevant
3. Consider adding request ID to error responses for better client-side debugging

Low Priority

1. Extract conn.__socket to a variable in the stale disconnect check for clarity
2. Add inline comments explaining the stale disconnect logic

Verdict

Recommendation: Approve with Minor Changes

The PR successfully implements request ID tracking with clean, type-safe code. The core implementation is solid, but would benefit from:

1. Test coverage for the new functionality
2. Clarification on the unrelated timeout change
3. Better documentation

The changes are well-structured and maintain consistency across all connection types. Once tests are added and the minor concerns addressed, this will be a solid improvement to observability.

---

Review generated for commit: dfa6543

[claude]

PR Review: Pass request ID from driver to conn

Summary

This PR implements request ID tracking from the driver level down to connections, enabling better correlation between underlying requests and connections. The changes are well-structured and improve observability.

Positive Observations

1. Consistent Naming Convention ✅

The refactoring from socketId to requestId is semantically more accurate:

• socketId implied it was specific to socket connections
• requestId correctly represents that this ID tracks individual requests (HTTP, SSE, WebSocket)
• The naming aligns with the architecture where each request gets a unique identifier

2. Clean API Changes ✅

The changes to engine/sdks/typescript/runner/src/mod.ts properly thread the requestId parameter through:

fetch: (runner: Runner, actorId: string, requestId: protocol.RequestId, request: Request)
websocket: (runner: Runner, actorId: string, ws: any, requestId: protocol.RequestId, request: Request)

3. Improved Observability ✅

Adding requestId and driver fields to connection inspection data (lines 235-238 in instance.ts) enhances debugging capabilities:

requestId: conn.__socket?.requestId,
driver: conn.__driverState
    ? getConnDriverKindFromState(conn.__driverState)
    : undefined,

4. Consistent ID Generation ✅

Using crypto.randomUUID() for request IDs is appropriate and consistent with UUID v4 conventions mentioned in CLAUDE.md.

5. Proper Propagation ✅

The requestId is correctly passed through all connection creation paths:

• HTTP requests
• SSE connections
• WebSocket connections (both connect and raw)
• Internal executeAction calls

Issues & Concerns

1. ⚠️ Grafana Dashboard Changes

Location: All guard.json and tokio.json files across multiple docker environments

Issue: The Grafana dashboard JSON formatting changes (compacting arrays) appear to be incidental formatting changes unrelated to the PR's purpose. According to CLAUDE.md:

Do not make changes to docker/dev* configs. Instead, edit the template in docker/template/ and rerun (cd docker/template && pnpm start). This will regenerate the docker compose config for you.

Recommendation:

• If these changes are intentional, they should be isolated to a separate PR
• If unintentional, verify whether the template was properly regenerated
• The formatting changes add noise to the PR review (148 additions / 300 deletions mostly from these files)

2. ⚠️ Unrelated Configuration Change

Location: examples/kitchen-sink/src/backend/actors/demo.ts:160

-    sleepTimeout: 2000,
+    sleepTimeout: 10_000,

Issue: Changing the sleep timeout from 2s to 10s appears unrelated to request ID tracking.

Recommendation:

• If this change is necessary, it should be explained in the PR description
• Consider moving to a separate PR if unrelated to the main feature

3. 🔍 Potential Type Safety Improvement

Location: rivetkit-typescript/packages/rivetkit/src/actor/conn-socket.ts

export interface ConnSocket {
-   socketId: string;
+   requestId: string;
}

Observation: The requestId is typed as string, but in many places it's used as protocol.RequestId (which appears to be ArrayBuffer).

Evidence: In actor-driver.ts:149, the signature shows:

getActorHibernationConfig: (actorId: string, requestId: ArrayBuffer, request: Request)

And in tunnel.ts, requestId is typed as protocol.RequestId:

async #handleWebSocketOpen(requestId: protocol.RequestId, ...)

Recommendation: Verify type consistency. If protocol.RequestId is ArrayBuffer, the ConnSocket interface may need adjustment, or conversion using bufferToString() should be explicit and well-documented.

4. 📝 Missing Documentation

Issue: No inline documentation explains:

• Why request IDs are needed
• How they differ from connection IDs
• When to use requestId vs connId vs socketId (if any remain)

Recommendation: Add JSDoc comments to key interfaces/functions explaining the purpose of request ID tracking.

Testing Considerations

Missing Test Coverage

The PR doesn't include test updates. Consider adding tests for:

1. Request ID Propagation: Verify requestId flows correctly through all connection types
2. Stale Disconnect Detection: The logic at instance.ts:884-896 that prevents stale disconnect events should have test coverage
3. Hibernation: Verify request ID matching works correctly for hibernatable WebSockets

Suggested Test Scenarios

// Test that stale disconnect events are ignored
// Test that requestId is properly logged for debugging
// Test that requestId matches between connection creation and disconnection

Performance Considerations

✅ No significant performance concerns identified. The changes primarily involve passing an additional parameter and storing an additional field, which has minimal overhead.

Security Considerations

✅ No security concerns identified. Request IDs are UUIDs generated with crypto.randomUUID(), which is cryptographically secure.

Code Quality

Strengths

• Consistent parameter naming throughout the codebase
• Proper use of structured logging (using tracing patterns mentioned in CLAUDE.md)
• Good error handling maintained

Minor Suggestions

1. Consider extracting the conversion logic bufferToString(requestId) into a dedicated utility function for clarity
2. The dual representation of requestId (as string in some places, ArrayBuffer in others) could be more explicitly documented

Final Recommendation

Conditional Approval ✅ - Approve after addressing the following:

Required Changes

1. ❗ Clarify or remove the Grafana dashboard formatting changes
2. ❗ Explain or remove the unrelated sleepTimeout change

Recommended Changes

1. ⚡ Add test coverage for request ID propagation
2. 📚 Add documentation/comments explaining the request ID system
3. 🔍 Verify and document the type consistency for requestId (string vs ArrayBuffer)

Summary

The core functionality is solid and improves observability by enabling correlation between requests and connections. The implementation follows repository conventions and maintains code quality. However, the PR includes some unrelated changes that should be addressed before merging.

The changes successfully achieve the stated goal of passing request IDs from driver to connection level for better request correlation.

[claude]

Code Review

This PR refactors the connection tracking system to use requestId instead of socketId throughout the codebase. This improves observability by allowing correlation between driver-level requests and connection-level operations.

Strengths

1. Consistent naming - The rename from socketId to requestId better reflects the actual purpose
2. Improved observability - Adding requestId to inspector data and logs helps with debugging
3. Type safety maintained - TypeScript types are preserved throughout
4. Comprehensive refactor - All affected files updated consistently

Code Quality

Good patterns:

• Parameter order is consistent across functions
• Stale disconnect event handling in instance.ts:877-896 is well-implemented
• Proper structured logging with eventRequestId and currentRequestId fields

Minor issues:

• instance.ts:254 has redundant requestId: requestId (could use property shorthand)
• router-endpoints.ts:186 has same redundancy

Potential Issues

1. Type safety in tunnel.ts - Verify protocol.RequestId type matches usage
2. Ensure inspector UI handles undefined requestId and driver values gracefully
3. Verify SSE reconnections work correctly with new requestId system

Performance

• bufferToString conversion in actor-driver.ts:460 converts to base64 - verify not in hot paths
• Inspector overhead is minimal and worth it for observability

Questions

• Sleep timeout change in demo.ts from 2000ms to 10000ms - is this related to the refactor?
• Grafana dashboard JSON formatting changes - consider separate commit?

Recommendations

LGTM with minor suggestions. Before merging:

1. Verify the sleep timeout change in demo.ts is intentional
2. Consider using property shorthand for requestId: requestId
3. Add brief explanation in PR description about debugging benefits

The core logic is sound and maintains all existing functionality while improving traceability.

[graphite-app]

Merge activity

• Nov 4, 8:09 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 4, 8:11 PM UTC: CI is running for this pull request on a draft pull request (#3349) due to your merge queue CI optimization settings.
• Nov 4, 8:41 PM UTC: The Graphite merge queue removed this pull request due to removal of a downstack PR #3342.
• Nov 4, 8:49 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 4, 8:50 PM UTC: CI is running for this pull request on a draft pull request (#3352) due to your merge queue CI optimization settings.
• Nov 4, 8:52 PM UTC: Merged by the Graphite merge queue via draft PR: #3352.

[claude]

PR Review: Pass Request ID from Driver to Conn

Overview

This PR successfully implements request ID propagation from the driver layer through to connections, enabling correlation between underlying requests and connections for improved observability.

Positive Aspects

1. Clean API Design: The renaming from socketId to requestId in ConnSocket is semantically clearer and better reflects the purpose - correlating requests rather than identifying sockets.

2. Consistent Propagation: The request ID flows cleanly through the entire stack:

   • Generated in actor-driver.ts (line 460)
   • Passed through runner callbacks (#runnerFetch, #runnerWebSocket)
   • Propagated to connection creation in router-endpoints.ts
   • Stored in ConnSocket interface

3. Stale Event Protection: The disconnect handler properly validates request IDs to prevent stale disconnect events (instance.ts:883-897), which is good defensive programming.

4. Inspector Integration: Added requestId and driver fields to the inspector's connection data (instance.ts:235-237), improving debuggability.

Code Quality Observations

Good Practices

• Type safety maintained with protocol.RequestId type
• Proper logging with structured fields (eventRequestId, currentRequestId)
• The helper function generateConnRequestId() provides a clear API

Minor Issues

1. Inconsistent Parameter Naming (rivetkit-typescript/packages/rivetkit/src/drivers/engine/actor-driver.ts:440):

   async #runnerFetch(
       _runner: Runner,
       actorId: string,
       _requestIdBuf: ArrayBuffer,  // ← Named with underscore prefix but used
       request: Request,
   ): Promise<Response>

   The parameter _requestIdBuf has an underscore prefix suggesting it's unused, but it's actually intended to be used. This parameter should either:

   • Be used and renamed without the underscore prefix, OR
   • If truly unused for fetch, add a comment explaining why

2. Missing Type Import (engine/sdks/typescript/runner/src/tunnel.ts:453):

   async #handleWebSocketOpen(
       requestId: protocol.RequestId,  // ← Good! Type used here

   The type is used correctly, but ensure protocol.RequestId is properly exported.

3. Sleep Timeout Change (examples/kitchen-sink/src/backend/actors/demo.ts:160):

   options: {
       sleepTimeout: 10_000,  // Changed from 2000
   }

   This appears to be an unrelated change. While it may have been needed for testing, it should either:

   • Be documented in the PR description, OR
   • Be moved to a separate PR

Potential Concerns

1. Database Schema Impact: The change to ConnSocket interface is good, but verify that any persisted connection data doesn't rely on the old socketId field. If connections are serialized/deserialized across hibernation, ensure backward compatibility.

2. Request ID Generation Timing: In executeAction (instance.ts:254), a request ID is generated for internal actions. Consider if this could cause confusion in logs when distinguishing between real external requests and internal action invocations.

3. Grafana Dashboard Changes: The JSON formatting changes in the Grafana dashboards (guard.json, tokio.json) appear to be auto-generated reformatting. Per CLAUDE.md guidelines about docker/dev* configs, verify these were generated via the template system and not manual edits.

Security Considerations

No security issues identified. The request ID is a UUID used for correlation only, not authentication/authorization.

Performance Considerations

No performance concerns. The changes add minimal overhead (UUID generation and passing an additional parameter).

Test Coverage

Recommendation: Add tests to verify:

1. Request ID propagation through the full stack
2. Stale disconnect event prevention with mismatched request IDs
3. Inspector data includes correct request ID and driver information

Example test case:

test("should ignore stale disconnect events with mismatched request IDs", async () => {
    // Create connection with requestId1
    // Reconnect same connection with requestId2
    // Trigger disconnect with requestId1
    // Assert connection still active
});

Recommendations

1. Address the _requestIdBuf parameter naming in actor-driver.ts:440
2. Document or separate the sleep timeout change in demo.ts
3. Add test coverage for request ID correlation
4. Verify backward compatibility for any persisted connection data

Conclusion

This is a solid observability improvement. The code quality is good with clear intent and proper error handling. The main recommendations are minor cleanup items and test coverage additions.

Approval Status: ✅ Approved with minor suggestions

---

Generated by Claude Code

[claude]

Code Review - PR #3318

Summary

This PR adds request ID tracking through the connection stack, allowing correlation between driver-level requests and connections. The changes propagate the request ID from the runner tunnel layer through to connection objects.

---

Positive Observations

✅ Good architectural improvement: Passing request IDs through the stack enables better observability and debugging by correlating requests with connections.

✅ Consistent naming: The rename from socketId to requestId is more accurate since it tracks requests, not just sockets.

✅ Proper propagation: The request ID flows cleanly through:

• tunnel.ts → mod.ts (runner config) → actor-driver.ts → router-endpoints.ts → conn.ts

✅ Non-breaking for generated files: Grafana dashboard changes are just formatting (multi-line arrays to single-line), which is acceptable for generated configs.

---

Issues & Concerns

1. Potential Breaking Change ⚠️

The function signature changes in engine/sdks/typescript/runner/src/mod.ts are breaking:

// Before
fetch: (runner: Runner, actorId: string, request: Request) => Promise<Response>
websocket?: (runner: Runner, actorId: string, ws: any, request: Request) => Promise<void>

// After  
fetch: (runner: Runner, actorId: string, requestId: protocol.RequestId, request: Request) => Promise<Response>
websocket?: (runner: Runner, actorId: string, ws: any, requestId: protocol.RequestId, request: Request) => Promise<void>

Impact: Any external consumers of the runner SDK will need to update their code.
Recommendation: Consider if this warrants a major version bump or if backward compatibility is needed.

2. Unused Parameter in actor-driver.ts:437 ⚠️

async #runnerFetch(
    _runner: Runner,
    actorId: string,
    _requestIdBuf: ArrayBuffer,  // ⚠️ Unused!
    request: Request,
): Promise<Response>

The _requestIdBuf parameter is received but never used. This suggests incomplete implementation.

Recommendation:

• If the request ID should be logged or passed downstream, implement that logic
• If it's for future use, add a comment explaining why it's not currently used
• Consider if this should be added to debug/trace logging

3. Inconsistent Parameter Naming

In tunnel.ts:453, the parameter is typed as protocol.RequestId:

async #handleWebSocketOpen(
    requestId: protocol.RequestId,  // typed
    open: protocol.ToClientWebSocketOpen,
)

But in tunnel.ts:210, it's just a string:

async #fetch(actorId: string, requestId: protocol.RequestId, request: Request)

While protocol.RequestId is likely just ArrayBuffer (based on usage with bufferToString), the inconsistency should be clarified.

Recommendation: Use the type consistently throughout, or add type aliases for clarity.

4. Missing Context in Debug Logs 💡

The instance.ts debug log at line 883 mentions the request ID but doesn't include the actor ID:

this.#rLog.debug({
    msg: "ignoring stale disconnect event",
    connId: conn.id,
    eventRequestId: requestId,
    currentRequestId: conn.__socket.requestId,
    // Missing: actorId for better correlation
});

Recommendation: Add actorId: this.id to help correlate logs across the system.

5. Unrelated Change ❓

examples/kitchen-sink/src/backend/actors/demo.ts:160

-   sleepTimeout: 2000,
+   sleepTimeout: 10_000,

This appears unrelated to request ID tracking. Was this intentional or left in by mistake?

Recommendation: If intentional, mention in the PR description. If not, revert it.

---

Performance & Security

✅ No performance concerns: Adding a request ID parameter has negligible overhead.
✅ No security issues identified: Request IDs are used for correlation only, no sensitive data exposure.

---

Testing Considerations

⚠️ Missing test updates: No test file changes are included in this PR.

Recommendations:

1. Add unit tests verifying request ID propagation through the stack
2. Add integration tests ensuring __connDisconnected correctly handles stale disconnect events with mismatched request IDs
3. Test that debug logs include the expected request ID fields

---

Code Quality

According to CLAUDE.md conventions:

✅ Uses structured logging with tracing patterns
✅ Follows TypeScript conventions
⚠️ Check if formatting matches project standards (the Grafana JSON changes suggest auto-formatting was applied)

---

Summary Recommendations

Must Address:

1. ✅ Document the breaking changes in runner SDK interfaces
2. ✅ Either use or document why _requestIdBuf is unused in #runnerFetch
3. ✅ Clarify if the sleepTimeout change in the example is intentional

Should Consider:

1. Add tests for request ID tracking
2. Add actor ID to debug logs for better correlation
3. Consider adding request ID to more debug/trace logs throughout the connection lifecycle

Nice to Have:

1. Add inline comments explaining the request ID flow for future maintainers
2. Document in the PR description how this improves observability

---

Overall, this is a solid improvement for observability. The main concerns are the breaking changes and ensuring the request ID is actually utilized where it's passed.