Add browser-based Valdi debugger - #154
Conversation
clholgat
left a comment
There was a problem hiding this comment.
Took a pass with a focus on the loopback server, the browser client, and the shared valdi_core serialization. Overall this is a solid, well-engineered PR — the security perimeter (loopback bind, Host allowlist, Origin + Sec-Fetch-Site CSRF, path-traversal containment, streaming body-size cap) is above average, the browser client is well-escaped with no live XSS, and the component-debug serialization is genuinely bounded and zero-cost on the non-debug path. A few small things worth fixing before merge, inline. No blockers.
|
| Test Suite | Result |
|---|---|
| Valdi Smoke Tests | ✅ success |
| valdi_web Integration Test | ✅ success |
| Linux: Hotreload Smoke | ✅ success |
| Linux: Registry Validation | ✅ success |
| Linux: C++ Tests | ❌ failure |
| Linux: Module Tests | ✅ success |
| Test Coverage Delta | ✅ success |
| Snapshot Tests | ✅ success |
| macOS: C++ & Platform Tests | ❌ failure |
| API Surface Check | ✅ success |
| Linux: Build Compiler | ✅ success |
| Linux: Build & Export | ✅ success |
Some tests failed. Please check the workflow logs for details.
🚀 Bazel remote cache is now enabled - future builds will be faster!
Workflow: Valdi CI
|
Thanks @bjdodson-openai — all review feedback is addressed and the fixes look good. I'm pulling this into our internal monorepo now for further internal testing. (Heads up: the remaining red C++ Tests check is a pre-existing flaky death test — |
Description
Adds a packaged, browser-based Valdi debugger available through
valdi debugger.The debugger provides:
Component debug data is explicitly opt-in, bounded, and excluded from existing
valdi inspect treerequests by default. The local server is restricted to loopback and validates Host, Origin, request size, action names, ports, and projected media sources.This also adds CLI packaging, documentation, CI path coverage, and regression tests.
Type of Change
Testing
bazel test //...)Testing Details
npm test: 135 specs passedbazel query //...passedModuleLoader.preloadBatchsuite.Checklist
Related Issues
N/A
Additional Context
This PR intentionally establishes the native debugger foundation only.
Deferred follow-ups: