feat(rivet-engine): udb key parser

[NathanFlurry]

No description provided.

[vercel]

Deployment failed with the following error:

Too many requests - try again in 3 minutes (more than 120, code: "api-deployments-flood-pro").

Learn More: https://vercel.com/docs/limits#rate-limits

[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

• feat(rivetkit): add ability to destroy actors #3458 : 2 dependent PRs (#3459 , #3462 )
• feat(rivet-engine): udb key parser #3457 👈 (View in Graphite)
• fix(rivetkit): fix zod not validating body if content-type header not provided #3456
• fix(rivetkit): fix onError handler not working #3455
• chore(site): tweak wording #3454
• chore(site): fix generating examples #3453
• chore: create branch for v3 website changes on v2 #3304
• chore(website): flatten actors docs structure #3280
• chore(rivetkit): switch dynamic node imports to use require #3461
• chore(rivetkit-typescript): remove dependency on node modules #3460
• feat(site): add typedoc docs #3446
• chore(rivetkit): remove deprecated packages #3447
• chore(rivetkit): add support for conn params on onRequest and onWebSocket #3445
• feat(rivetkit): add asyncapi spec #3442
• chore(rivetkit): add actor router to the openapi spec #3441
• fix(rivetkit): skip sending RivetKit messages to conns that do not support it #3440
• fix(rivetkit): fix race condition with websocket open events #3439
• fix(rivetkit): standardize error metadata return type #3438
• fix(rivetkit): remove incorrect getEndpoint call in metadata handler #3433 : 1 other dependent PR (#3451 )
• feat(rivetkit): add json protocol support #3432
• chore(rivetkit): add required zod validation for json encoding #3431
• chore(rivetkit): add listing actors by only name to manager api #3430
• refactor(rivetkit): extract WebSocket protocol parsing to shared utility #3429
• refactor(rivetkit): restructure connection lifecycle contexts #3428
• chore(rivetkit): add WebSocketContext and RequestContext #3427
• chore(rivetkit): add conn type #3426
• chore(rivetkit): add connection for raw http #3425
• chore(rivetkit): rename onFetch to onRequest #3424
• refactor(rivetkit): add state manager for conn with symbol-based access #3423
• fix(rivetkit): use loggerWithoutContext for websocket error when actor undefined #3412
• feat(rivetkit): add GET /actors/names #3411
• feat(api-public): expose actor kv api #3410
• chore(rivetkit): auto-generate inspector token for engine driver #3409
• chore(rivetkit): close websockets on out of sequence messages #3404
• fix(rivetkit): properly apply clientEndpoint from metadata lookup #3402
• fix(rivetkit): properly handle msgIndex for hibernatable websocket reconnection #3401
• chore(rivetkit): rename onstart/onstop to onwake/onsleep #3422
• chore(rivetkit): split ActorInstance logic in to multiple classes #3421
• chore(rivetkit): move conns to separate persisted kv keys #3420
• chore(rivetkit): remove deprecated conn tokens & sse support #3419
• 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]

PR Review: UDB Key Parser

Thanks for this contribution! This adds a useful CLI utility for decoding UniversalDB keys and parsing transaction conflict logs.

Strengths

1. Clear Purpose - The tool provides valuable debugging utilities for UDB keys and transaction conflicts
2. Good Error Handling - Proper use of anyhow::Context for error messages with helpful context
3. Follows Project Conventions - Uses anyhow::* imports correctly, structured logging in transaction_conflict_tracker.rs follows the pattern, hard tabs for formatting

Code Quality Observations

udb_keys.rs:

1. Line 36-38 - Redundant code, can simplify by returning decode_array(array) directly
2. Line 54-61 - Using match when ? operator would suffice
3. Line 84 - Consider using HashMap at the top-level imports instead of inline per CLAUDE.md
4. Logfmt Parser (lines 83-117) - The hand-rolled logfmt parser is fragile. Consider using a logfmt parsing crate like logfmt or logfmt-parse for robustness
5. Line 77 - Using contains() for log detection is inefficient for large files

transaction_conflict_tracker.rs:

6. Excellent Addition - The structured logging at lines 67-79 perfectly complements the new parser tool
7. Follows conventions - Lowercase message with structured fields

Potential Issues

1. Memory Concern - parse_conflicts() loads entire log files line-by-line, fields HashMap is recreated for every line
2. Error Recovery - When decoding fails, errors are printed but parsing continues (appropriate for debugging tool)
3. No Input Validation - The --file parameter doesn't check if file exists before attempting to open

Test Coverage

Missing unit tests for the new functionality. Consider adding:

• Tests for decode_array() with valid and invalid JSON
• Tests for decode_from_logfmt() with various input formats
• Tests for the logfmt parser with edge cases

Performance Considerations

• File reading is buffered (good)
• String allocations in logfmt parser acceptable for CLI tool
• Consider adding --limit flag to stop after N conflicts for very large logs

Security

• No security concerns for a local debugging tool
• File access is properly handled with error context

Minor Suggestions

1. Documentation - Add doc comments for public functions
2. CLI UX - Consider adding --verbose flag
3. Output Format - Consider adding --json output format

Test Script Changes

The changes to scripts/tests/actor_spam.ts and scripts/tests/utils.ts look good - making the key parameter optional is sensible.

Conclusion

This is a solid debugging utility that will be valuable for troubleshooting transaction conflicts. Main recommendations:

1. Add unit tests for the core parsing logic
2. Consider using a logfmt parsing library for robustness
3. Simplify error handling in a few places
4. Add top-level imports per project conventions

The transaction conflict logging addition is excellent and will make this tool very useful in production debugging scenarios.

Recommendation: Approve with minor improvements suggested above. The tool is functional and follows most project conventions.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Nov 13, 2025 3:24am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 3:24am
rivet-inspector	Ignored	Preview		Nov 13, 2025 3:24am
rivet-site	Ignored	Preview		Nov 13, 2025 3:24am

[claude]

PR Review: feat(rivet-engine): udb key parser

Summary

This PR adds a UDB key parsing utility for debugging and analyzing UniversalDB transaction conflicts. The implementation includes a CLI tool to decode UDB keys from byte arrays and parse transaction conflict logs.

Code Quality & Best Practices

Positive Aspects

• Good use of structured error handling with anyhow::Context
• Clear CLI interface with well-documented subcommands
• Proper separation of concerns (decode vs parse functionality)
• Good code organization following existing patterns

Issues & Suggestions

1. Import Pattern Inconsistency (Critical per CLAUDE.md)

Location: engine/packages/engine/src/commands/udb_keys.rs:6

The code uses: use anyhow::{Context, Result, bail};

But according to CLAUDE.md, the pattern should be: use anyhow::*;

Recommendation: Change to wildcard import to match project conventions.

2. Manual Logfmt Parser (Code Quality)

Location: engine/packages/engine/src/commands/udb_keys.rs:83-117

The manual logfmt parsing implementation is error-prone and doesn't handle edge cases properly:

• Doesn't handle escaped equals signs in values
• Doesn't handle quotes within quoted values
• May have issues with complex nested structures

Recommendation: Consider using a proper logfmt parsing library (e.g., logfmt crate) instead of manual parsing.

3. Structured Logging Enhancement

Location: engine/packages/universaldb/src/driver/rocksdb/transaction_conflict_tracker.rs:67-79

The structured logging addition is excellent and follows CLAUDE.md guidelines! The use of ? debug formatting for byte arrays and separate fields for versions is correct.

Test Coverage

No tests detected for the new udb_keys module.

Recommendations:

1. Add unit tests for the decode_array function with various byte patterns
2. Add tests for the logfmt parser with edge cases
3. Add integration tests with sample log files
4. Test error cases (malformed JSON, invalid byte arrays, etc.)

Overall Assessment

Strengths:

• Well-structured debugging utility
• Good use of existing SimpleTuple infrastructure
• Helpful transaction conflict logging enhancement
• Clear CLI interface

Areas for Improvement:

• Fix import patterns per CLAUDE.md conventions
• Consider using a proper logfmt parsing library
• Add comprehensive test coverage
• Minor refactoring for more idiomatic Rust

Recommendation: Approve with minor changes

The core functionality is solid and will be valuable for debugging. The suggested improvements would enhance robustness and maintainability, but none are blocking issues.

[claude]

PR Review: UDB Key Parser

I've reviewed this PR and have the following feedback:

Positive Aspects

✅ Good Feature Addition: The UDB key parser provides a valuable debugging tool for analyzing transaction conflicts and decoding UniversalDB keys.

✅ Code Organization: The new command is well-integrated into the existing CLI structure, following the established patterns in the codebase.

✅ Documentation: Good use of doc comments on the CLI arguments explaining usage.

✅ Error Handling: Proper use of anyhow::Context for adding context to error messages throughout the code.

✅ Logging Enhancement: The addition of structured logging in transaction_conflict_tracker.rs:67-79 is excellent - it logs all the necessary information to debug conflicts.

Issues and Concerns

1. Logfmt Parser Implementation (Medium Priority)

The hand-rolled logfmt parser in udb_keys.rs:83-116 has potential bugs:

• Line 92: The condition for handling quotes doesn't handle escaped quotes within quoted values
• Line 90-111: The parser doesn't properly handle edge cases like values containing = characters inside quotes, empty values, or trailing spaces in keys

Recommendation: Consider using a proper logfmt parsing library like logfmt or logfmt-rs instead of a hand-rolled parser. If you must use a custom parser, add unit tests covering edge cases.

2. Missing Tests (High Priority)

The new module has zero test coverage. Given this is a debugging/operational tool, tests are important to ensure it correctly parses the logs it's meant to parse.

Recommendation: Add tests for decode_array(), decode_from_logfmt(), parse_conflicts() with sample log files, and logfmt parser edge cases.

3. Error Recovery in parse_conflicts() (Low Priority)

In udb_keys.rs:149-171, if decoding fails for one key, the error is printed but processing continues. This is good for robustness, but there's no indication in the summary output that some keys failed to decode.

Recommendation: Track failed decodes and include a count in the final summary.

4. Synchronous I/O in Async Context (Medium Priority)

The execute() method at lib.rs:52 calls opts.execute() which is synchronous, while all other command executors are async. The udb_keys command does synchronous file I/O in parse_conflicts().

Recommendation: For consistency with other commands, consider making execute() async and using tokio::fs for file operations.

5. Test Script Changes (Question)

The changes to scripts/tests/actor_spam.ts and scripts/tests/utils.ts seem unrelated to the UDB key parser feature. Are these changes intentional for this PR, or should they be in a separate commit/PR?

Security Considerations

✅ No security concerns identified. The tool only reads local log files, decodes data but doesn't modify any state, and uses safe parsing with proper error handling.

Performance Considerations

✅ Performance is appropriate for a CLI debugging tool - the logfmt parser processes line-by-line which is good for large files.

Recommendations Summary

Must Fix Before Merge:

• Add tests for the new module (especially the logfmt parser)
• Either fix the logfmt parser bugs or use a proper library

Should Fix:

• Track and report failed key decodes in summary
• Clarify why test script changes are included in this PR

Nice to Have:

• Make the command async for consistency
• Add test fixtures with sample conflict logs

Overall, this is a useful debugging feature, but needs test coverage before merging.

[claude]

Code Review

I've reviewed PR #3457 and here's my feedback:

Summary

This PR adds a useful debugging utility for UniversalDB keys and transaction conflicts. The implementation adds a new CLI command UdbKeys with two subcommands:

1. Decode - Decodes UDB keys from JSON byte arrays
2. ParseConflictLogs - Parses logfmt logs to display transaction conflicts

Positive Aspects ✅

1. Well-structured CLI Interface

• Clean use of clap with clear documentation on command arguments
• Good separation of concerns between decode and parse logic

2. Enhanced Debugging Capability

• The transaction conflict tracker now logs structured data (transaction_conflict_tracker.rs:67-79)
• The new parser makes analyzing conflicts much easier
• Nice formatted output with visual separators

3. Code Quality

• Proper error handling with context messages
• Follows existing patterns in the codebase (e.g., using anyhow::*)
• Good use of structured logging with tracing

Issues & Concerns 🔍

1. Custom logfmt Parser Has Bugs ⚠️
The hand-rolled logfmt parser in udb_keys.rs:83-117 has several issues:

• Doesn't handle escaped characters properly: The parser tracks escaped but doesn't actually use backslash escaping correctly. It should skip the backslash and keep the next character.
• Doesn't handle edge cases: Empty values, values with = signs, nested quotes
• Not production-grade: logfmt has nuances that are easy to get wrong

Recommendation: Consider using a proper logfmt parser library like logfmt or logfmt-parser instead of rolling your own. This would be more robust and maintainable.

2. File I/O Without Path Validation 🔒
In parse_conflicts() (udb_keys.rs:66):

• The file path is passed directly to fs::File::open() without any validation
• While this is a CLI tool, it could be more user-friendly to check if the path exists first and give a clearer error message

3. Performance Consideration ⚡
In parse_conflicts() (udb_keys.rs:90-112):

• The character-by-character parsing with .clone() on every HashMap insert could be optimized
• For large log files, this could be slow
• Consider using string slices or a proper parser combinator

4. Potential Issue with Nested Decoding
In decode_from_logfmt() (udb_keys.rs:185-199):

• Silently ignores failed decoding by using print! before the decode attempt
• If decode fails, the "Start:" or "End:" label is printed without a value, which could be confusing
• Consider buffering the output or handling errors more gracefully

5. Test Script Changes 📝
The changes to scripts/tests/actor_spam.ts and scripts/tests/utils.ts:

• Adds an optional withKey parameter defaulting to true
• actor_spam.ts explicitly passes false
• This seems unrelated to the main purpose of the PR (UDB key parsing)
• Question: Is this intentional or should this be in a separate PR? The PR title doesn't mention actor tests.

Suggestions for Improvement 💡

1. Add unit tests for the decode functions

   • Test with various key types
   • Test edge cases in logfmt parsing
   • Test malformed input

2. Consider adding a --json output flag for machine-readable output, which would be useful for tooling

3. Document the expected log format in comments or help text, so users know what to pipe in

4. Add usage examples in the command help text

5. The logging in transaction_conflict_tracker.rs is good, but consider using tracing::debug! instead of tracing::info! for high-frequency logs to avoid log spam in production

Security Considerations 🔐

• File path traversal: While this is a CLI tool, ensure it's documented that users should only point it at trusted log files
• No major security concerns for a debugging utility

Performance Considerations 🚀

• The decode operations are on-demand and not in the hot path
• The transaction conflict tracker logging adds minimal overhead (just a single log line)
• No concerns for the intended use case

Test Coverage 🧪

• Missing: No unit tests for the new functionality
• Recommendation: Add tests for:
  • decode_array() with various byte arrays
  • decode_from_logfmt() with different value formats
  • The logfmt parser edge cases

Conclusion

This is a useful debugging tool that will help diagnose transaction conflicts. The main concerns are:

1. The custom logfmt parser implementation (use a library instead)
2. The unrelated test script changes
3. Missing test coverage

Recommendation:

• ⚠️ Request changes for the logfmt parser issue
• Consider splitting out the test script changes if they're unrelated
• Add basic unit tests before merging

Overall the core functionality is solid and well-structured. With the logfmt parser improvement, this would be a great addition to the debugging toolkit.