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(rivetkit): switch dynamic node imports to use require #3461 : 1 other dependent PR (#3280 )
• 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 5:13pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 5:13pm
rivet-inspector	Ignored	Preview		Nov 13, 2025 5:13pm
rivet-site	Ignored	Preview		Nov 13, 2025 5:13pm

[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.

[claude]

PR Review: UDB Key Parser

Thanks for this PR! This adds a useful debugging tool for decoding UniversalDB keys and parsing transaction conflict logs.

Positive Aspects

1. Good Use of Existing Types: Leverages the existing SimpleTuple infrastructure from util/udb.rs for consistent key decoding
2. Clear CLI Structure: Well-organized Clap commands with helpful documentation
3. Error Handling: Proper use of anyhow with context for error messages
4. Logging Enhancement: The transaction conflict tracker logging is very useful for debugging

Potential Issues

1. Logfmt Parser Implementation (Lines 83-117): The custom logfmt parser has issues with escaped quotes, backslash escaping, and edge cases. Consider using a proper logfmt parsing library instead.

2. Missing Import: std::collections::HashMap is used on line 84 but not imported at the top of the file (per CLAUDE.md guidelines).

3. Documentation: Line 53 mentions foundationdb tuple unpacking but this is UniversalDB - minor inconsistency.

4. Test script changes: The changes to scripts/tests/actor_spam.ts and scripts/tests/utils.ts appear unrelated to the UDB key parser feature. Were these meant for a different PR?

Testing

Missing test coverage for the new functionality. Consider adding unit tests for decode_array, the logfmt parser, and integration tests for the CLI commands.

Recommendations

1. High Priority: Replace custom logfmt parser with a proper library for robustness
2. Medium Priority: Add test coverage
3. Low Priority: Add example usage to help text

Summary

This is a useful debugging tool with a solid foundation. The main concern is the custom logfmt parser which could fail on edge cases. Otherwise, the code follows project conventions well!

[claude]

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

Summary

This PR adds a CLI utility for parsing and debugging UniversalDB (UDB) keys, along with enhanced logging for transaction conflicts. The implementation provides two main features:

1. Decoding byte arrays into human-readable UDB tuples
2. Parsing transaction conflict logs from logfmt files

Code Quality & Best Practices

Strengths

• Good code organization: The new command is properly integrated into the CLI structure
• Consistent error handling: Uses anyhow::Context appropriately for error propagation
• Follows repository patterns: Uses existing SimpleTuple infrastructure from util/udb.rs
• Helpful debugging tool: Addresses a real developer pain point for debugging transaction conflicts

Issues & Recommendations

1. Manual logfmt Parsing 🔴
The hand-rolled logfmt parser (lines 84-115) is fragile and doesn't handle edge cases properly:

• No handling of escaped quotes within values
• No proper handling of nested quotes
• May fail on complex field values

Recommendation: Use a battle-tested logfmt parsing library instead.

2. Structured Logging Enhancement ✅
Great work on the transaction conflict logging! This follows the structured logging guidelines perfectly with proper use of debug formatting for byte arrays.

3. Missing Documentation ⚠️
The new module lacks doc comments explaining its purpose and usage.

4. Test Script Changes ⚠️
The change to createActor(..., false) in actor_spam.ts isn't explained in the PR description. If this is fixing a bug or testing a specific scenario, it should be mentioned.

Potential Bugs

1. Quote Trimming Logic 🔴
Line 185: value.trim_matches('"') will incorrectly strip quotes from values that legitimately start/end with quotes.

Fix: Only strip if both start and end are quotes

2. Missing Field Validation ⚠️
The code doesn't validate that all expected fields exist before printing, which could silently skip important information.

Performance Considerations

1. Line-by-Line Processing ✅
The buffered reader approach is efficient for log files.

2. String Allocations 🟡
Multiple string allocations in parsing loop could be optimized for very large files, but acceptable for debugging purposes.

Security Concerns

1. File Path Validation 🟡
No validation of file paths or size limits. Consider warning if file is extremely large to prevent DoS via memory.

2. JSON Parsing ✅
Proper error handling prevents panics on malformed input.

Test Coverage

Missing Tests 🔴

• No unit tests for the logfmt parser
• No integration tests for the decode functionality
• No test cases for the new CLI commands

Recommendations:

1. Add unit tests for decode_array() with valid and invalid inputs
2. Add tests for the logfmt parser with edge cases
3. Add a sample log file fixture for integration testing

Additional Suggestions

1. CLI UX: Consider adding a --format flag for output (json, pretty, compact)
2. Performance: For large log files, consider streaming output rather than collecting all conflicts
3. Filtering: Add flags like --since, --until, --limit for log parsing

Summary

Strengths: Good debugging utility, follows repository conventions, solid integration with existing code

Main Concerns:

1. 🔴 Hand-rolled logfmt parser needs to be replaced with proper library
2. 🔴 Missing test coverage
3. ⚠️ Quote handling bug in decode_from_logfmt
4. ⚠️ Unclear connection between script changes and main feature

Verdict: Approve with requested changes. The core functionality is sound, but the logfmt parsing needs improvement and tests are required.

---

Review completed by Claude Code