Skip to content

donotmerge: insert and clear actor queue#5065

Draft
abcxff wants to merge 1 commit into
mainfrom
05-19-feat_inspector_insert_and_clear_actor_queue
Draft

donotmerge: insert and clear actor queue#5065
abcxff wants to merge 1 commit into
mainfrom
05-19-feat_inspector_insert_and_clear_actor_queue

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 19, 2026

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

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 19, 2026

🚅 Deployed to the rivet-pr-5065 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web Jun 5, 2026 at 3:22 pm
frontend-inspector ❌ Build Failed (View Logs) Web Jun 3, 2026 at 1:14 am
frontend-cloud ❌ Build Failed (View Logs) Web Jun 3, 2026 at 1:14 am
website 😴 Sleeping (View Logs) Web May 30, 2026 at 9:03 pm
ladle ✅ Success (View Logs) Web May 19, 2026 at 8:30 pm
mcp-hub ✅ Success (View Logs) Web May 19, 2026 at 8:20 pm

Copy link
Copy Markdown
Contributor Author

abcxff commented May 19, 2026

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 19, 2026

Code Review: PR #5065 — Insert and Clear Actor Queue

Two new inspector HTTP endpoints: DELETE /inspector/queue (clear all messages) and POST /inspector/queue (enqueue a message). The cross-layer wiring (Rust core → NAPI → TypeScript native + wasm runtimes) is consistent with how existing inspector routes are structured.


Bugs / Correctness

reset() doesn't wake waiting consumers (queue.rs:569)

After deleting all messages and resetting metadata, queue_notify.notify_waiters() is never called. Any actor code blocking in queue.next() / queue.next_batch() will keep waiting indefinitely rather than waking and observing an empty queue. Compare with every other write path (e.g. send() at line 346): they all fire self.0.queue_notify.notify_waiters() after the mutation. The reset should do the same, or this choice should at least be documented.

Non-atomic delete → metadata write

The reset deletes message KV entries, then writes the new metadata in a separate kv.put. A process crash between the two leaves persisted metadata with size > 0 pointing to keys that no longer exist. On restart, ensure_initialized() will reload the stale size from KV. The existing send() path uses kv.batch_put to commit message + metadata atomically — consider doing the same here (write metadata first and delete keys second so the count is always conservative, or batch the metadata into the last deletion call).

next_id resets to 0 → IDs repeat from 1

Resetting next_id: 0 means the next send() will issue ID 1 again. If any caller holds an old message ID (e.g. from a POST /inspector/queue response), it will silently alias a newly enqueued message. For an inspector-only tool this is low risk, but it deserves a comment.


Protocol / Architecture

Missing WebSocket inspector parity

Per docs-internal/engine/inspector-protocol.md:

When updating the WebSocket inspector, also update the HTTP endpoints.

The new DELETE and POST operations have no counterpart in the WebSocket inspector (src/inspector/client.browser.ts only handles QueueUpdated / QueueResponse read messages). If the browser-based inspector panel needs these actions, they must be wired into the WS transport as well.

Missing documentation updates

Also per inspector-protocol.md, new endpoints must update:

  • Tests in rivetkit-typescript/packages/rivetkit/tests/
  • website/src/metadata/skill-base-rivetkit.md
  • website/src/content/docs/actors/debugging.mdx (the "Get Queue Status" section at line 435 only documents the GET endpoint)

Minor / Style

InspectorEnqueueBody missing rename_all (mod.rs:244)

Other inspector request bodies (e.g. InspectorWorkflowReplayBody) use #[serde(rename_all = "camelCase")]. The two fields here are single-word so it doesn't change behavior, but the annotation should be added for consistency.

Number(message.id()) precision risk (native.ts:3567)

Message IDs are u64 in Rust. Number() loses precision above 2^53 - 1. The existing GET inspector handler returns m.id directly — the POST response should be consistent. For an inspector-only tool the practical risk is low, but the conversion is wrong in principle.

Error response inconsistency (inspector.rs:133)

The DELETE handler propagates errors with Err(error).context(...), which produces an unstructured 500. The GET handler wraps errors in inspector_anyhow_response() for a structured JSON error body. Consider using the same pattern for write endpoints.

name defaults to "" without validation

InspectorEnqueueBody::name defaults to an empty string via #[serde(default)]. An empty queue message name may be unexpected. A quick validation check or a comment noting this is intentional would help.


Summary

The core idea is clean and the cross-language layering is correct. The main items to address before merging: fire queue_notify on reset, fix the non-atomic delete/metadata ordering, add test coverage, and update the inspector docs per the protocol convention.

@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 3b46462 to a89784a Compare May 19, 2026 21:44
@abcxff abcxff force-pushed the 05-18-fix_rivetkit_wire_cancellationtoken_through_waitfornamesavailable_to_eliminate_kv_busy-polling branch from 73cbe67 to 9dc7e20 Compare May 21, 2026 00:35
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from a89784a to 95dd3b3 Compare May 21, 2026 00:35
@abcxff abcxff mentioned this pull request May 21, 2026
11 tasks
@abcxff abcxff changed the base branch from 05-18-fix_rivetkit_wire_cancellationtoken_through_waitfornamesavailable_to_eliminate_kv_busy-polling to graphite-base/5065 May 21, 2026 19:45
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 95dd3b3 to 3f807cd Compare May 21, 2026 19:45
@abcxff abcxff changed the base branch from graphite-base/5065 to 05-21-feat_workflow-engine_add_retryontimeout_opt-in_for_step_timeout_retries May 21, 2026 19:45
@abcxff abcxff force-pushed the 05-21-feat_workflow-engine_add_retryontimeout_opt-in_for_step_timeout_retries branch from f7b5398 to a7e1162 Compare May 22, 2026 04:17
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 3f807cd to 0295ef9 Compare May 22, 2026 04:17
@abcxff abcxff force-pushed the 05-21-feat_workflow-engine_add_retryontimeout_opt-in_for_step_timeout_retries branch from a7e1162 to fccc091 Compare May 22, 2026 05:08
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 0295ef9 to 81264d4 Compare May 22, 2026 05:08
@abcxff abcxff changed the title feat(inspector): insert and clear actor queue donotmerge: insert and clear actor queue Jun 1, 2026
@abcxff abcxff force-pushed the 05-21-feat_workflow-engine_add_retryontimeout_opt-in_for_step_timeout_retries branch from fccc091 to ea20605 Compare June 2, 2026 20:09
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 81264d4 to 4ea4f78 Compare June 2, 2026 20:09
@abcxff abcxff changed the base branch from 05-21-feat_workflow-engine_add_retryontimeout_opt-in_for_step_timeout_retries to graphite-base/5065 June 3, 2026 01:07
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 4ea4f78 to 9bef952 Compare June 3, 2026 01:13
@abcxff abcxff force-pushed the graphite-base/5065 branch from ea20605 to 85cc607 Compare June 3, 2026 01:13
@graphite-app graphite-app Bot changed the base branch from graphite-base/5065 to main June 3, 2026 01:13
@abcxff abcxff force-pushed the 05-19-feat_inspector_insert_and_clear_actor_queue branch from 9bef952 to 323844c Compare June 3, 2026 01:13
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