Skip to content

fix(pegboard): unwrap actor name metadata#4423

Merged
NathanFlurry merged 2 commits intomainfrom
metadata-double-nesting
Mar 17, 2026
Merged

fix(pegboard): unwrap actor name metadata#4423
NathanFlurry merged 2 commits intomainfrom
metadata-double-nesting

Conversation

@NathanFlurry
Copy link
Member

Description

Fix the Rust serverless metadata fetcher so it stores the inner actor metadata object instead of the full ActorName wrapper. This prevents /actors/names from returning metadata.metadata when actor names are sourced through the engine.

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?

Reviewed the workspace diff and traced the manager and engine codepaths to confirm the engine was wrapping ActorName twice. I started a targeted cargo test -p pegboard extract_actor_name_metadata run, but stopped it after the initial dependency build was taking too long.

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 railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4423 March 14, 2026 00:28 Destroyed
@railway-app
Copy link

railway-app bot commented Mar 14, 2026

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

Service Status Web Updated (UTC)
website 😴 Sleeping (View Logs) Web Mar 14, 2026 at 12:38 am
frontend-cloud 😴 Sleeping (View Logs) Web Mar 14, 2026 at 12:35 am
frontend-inspector 😴 Sleeping (View Logs) Web Mar 14, 2026 at 12:35 am
mcp-hub ✅ Success (View Logs) Web Mar 14, 2026 at 12:28 am
ladle ❌ Build Failed (View Logs) Web Mar 14, 2026 at 12:28 am

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 14, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4423

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4423

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4423

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4423

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4423

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4423

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4423

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4423

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4423

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4423

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4423

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4423

commit: 834741e

@claude
Copy link

claude bot commented Mar 14, 2026

PR Review: fix(pegboard): unwrap actor name metadata

The one-line fix is correct. Here is the analysis:

CHANGE ANALYSIS

The fix addresses a double-wrapping issue in the Rust serverless metadata fetcher. The /metadata endpoint returns actorNames where each entry has the shape { metadata: { ...actual metadata... } }. The old code stored the entire wrapper object as the metadata value; the new code correctly calls .get("metadata")?.as_object()? to extract only the inner object.

This is consistent with how the TypeScript side produces actor names (buildActorNames returns Record<string, { metadata: Record<string, unknown> }>) and how the Swift client deserializes it (MetadataActorName has a metadata field). The Rust consumer was the only side not unwrapping correctly.

CORRECTNESS

The filter_map chain handles missing or null metadata keys by silently dropping those entries, which is the right behavior for malformed data.

SUGGESTIONS

  1. Missing test - The PR author noted they stopped the test run early due to build time. A focused unit test in fetch.rs would be straightforward and guard against regression. The test could construct a HashMap from inline JSON with one valid entry and one missing-metadata entry, run the same filter_map logic, and assert only the valid ActorNameMetadata is produced with correct inner fields.

  2. Stronger typing (optional, out of scope) - The HashMap<String, serde_json::Value> deserialization target for actor_names in ServerlessMetadataPayload could be tightened to HashMap<String, ActorNameWrapper> with a small helper struct that has a metadata field. This would let serde handle the extraction declaratively and make the schema contract explicit in types rather than via manual JSON traversal. Not required for this fix but worth considering for a follow-up.

SUMMARY

The fix is correct and the diagnosis in the PR description is accurate. The only meaningful gap is test coverage. Approve with or without the test depending on team priority.

@claude
Copy link

claude bot commented Mar 14, 2026

PR Review: fix(pegboard): unwrap actor name metadata

Summary: Correct fix for a double-wrapping bug where the engine stored the entire ActorName wrapper object as metadata instead of extracting the inner metadata field.

Correctness

The fix is correct. The TypeScript buildActorNames function returns Record<string, { metadata: Record<string, unknown> }>, so each value has the shape { metadata: { ... } }. The old code called value.as_object()?.clone() which stored the entire wrapper object (resulting in metadata.metadata nesting at the API layer). The fix correctly extracts value.get("metadata")?.as_object()?.clone().

The updated comment on line 179 accurately describes the schema and is a helpful addition.

Concerns

1. Silent actor name drops without logging

The filter_map silently discards any actor name entry where metadata is missing or not an object. A malformed or unexpectedly shaped response will cause actors to disappear from /actors/names with no diagnostic output. Consider emitting a tracing::warn! when an entry is dropped to make future regressions easier to diagnose.

2. No test coverage

The PR author acknowledges stopping the test run early. No Rust unit tests exist for the pegboard_serverless_metadata_fetch parsing path. A focused unit test against a mock JSON payload covering both the happy path and the missing-metadata case would prevent this class of regression. The existing TypeScript driver tests (actor-metadata.ts) do not exercise the engine-side unwrapping.

3. Missing metadata treated as skip rather than empty map

The TypeScript ActorName interface declares metadata as a required field, but the Rust code silently drops the entire entry when it is absent. It is worth deciding explicitly whether the contract is "entry without metadata is invalid" (current behavior) or "entry without metadata gets an empty map" (unwrap_or_default()). If the former, a warn log (see point 1) makes that behavior observable. If the latter, a silent drop is wrong.

Verdict

The core fix is correct and appropriately scoped. Recommend: (1) add a tracing::warn! when an entry is dropped so future protocol mismatches surface visibly, and (2) add at least a minimal unit test for the metadata parsing logic before merging.

@NathanFlurry NathanFlurry merged commit f046d3e into main Mar 17, 2026
11 of 13 checks passed
@NathanFlurry NathanFlurry deleted the metadata-double-nesting branch March 17, 2026 21: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.

2 participants