Skip to content

Add browser-based Valdi debugger - #154

Open
bjdodson-openai wants to merge 2 commits into
Snapchat:mainfrom
bjdodson-openai:bjd/upstream-debugger-foundation
Open

Add browser-based Valdi debugger#154
bjdodson-openai wants to merge 2 commits into
Snapchat:mainfrom
bjdodson-openai:bjd/upstream-debugger-foundation

Conversation

@bjdodson-openai

Copy link
Copy Markdown
Collaborator

Description

Adds a packaged, browser-based Valdi debugger available through valdi debugger.

The debugger provides:

  • Live daemon target discovery and attachment
  • Native view hierarchy and inert HTML preview
  • Component ViewModel/state inspection
  • Root and element snapshots
  • Heap report export and runtime log streaming
  • Hermes CPU profiling
  • External action/state synchronization for automation

Component debug data is explicitly opt-in, bounded, and excluded from existing valdi inspect tree requests 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

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation improvement
  • Performance optimization
  • Test improvement
  • Other (new debugger feature)

Testing

  • Tests pass locally (bazel test //...)
  • Added/updated tests for changes (if applicable)
  • Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
  • Manual testing performed (describe below)

Testing Details

  • npm test: 135 specs passed
  • CLI build and package dry run passed
  • bazel query //... passed
  • The focused Valdi core target builds successfully, and all 15 new debugger serialization tests pass. The aggregate target remains red in the unchanged ModuleLoader.preloadBatch suite.
  • Manually started the built debugger server and verified:
    • JSON startup output and graceful shutdown
    • Packaged UI and security headers
    • Loopback/Host/Origin protections
    • Invalid action and port rejection
    • No local filesystem paths exposed through debugger status or log metadata

Checklist

  • Code follows project style guidelines
  • Documentation updated (if needed)
  • No breaking changes (or documented in description)
  • Commit messages follow conventional format
  • No secrets, API keys, or internal URLs included

Related Issues

N/A

Additional Context

This PR intentionally establishes the native debugger foundation only.

Deferred follow-ups:

  • Web inspection should land together with its first-party web bridge.
  • Renderer tracing requires the larger runtime/native instrumentation port first.
  • Input forwarding and data/network provider tooling should land with their corresponding runtime contracts and end-to-end tests.

@github-actions github-actions Bot added the area/docs Documentation label Aug 25, 2026

@clholgat clholgat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread npm_modules/cli/src/debugger/server.ts
Comment thread npm_modules/cli/debugger/index.html
@github-actions

Copy link
Copy Markdown

⚠️ Bazel & CI Test Results

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

@clholgat

Copy link
Copy Markdown
Collaborator

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 — AsyncStrictModeSyncCallAssertsOnMainThread hanging on fork() in a threaded context and timing out — unrelated to these changes.) I'll follow up here with anything internal testing surfaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants