Skip to content

feat(cli): stream product frames to a wire debugger behind --debugger - #656

Open
decrypto21 wants to merge 1 commit into
mainfrom
nidish/debugger-cli-flag
Open

decrypto21 wants to merge 1 commit into
mainfrom
nidish/debugger-cli-flag

Conversation

@decrypto21

@decrypto21 decrypto21 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Adds a --debugger flag so the headless truapi-host CLI streams its product frames to the wire debugger.

Why: the debugger only ever sees the browser host. wasm.rs was the sole non-test set_debug_sink caller, so every frame the CLI serves was unobservable, including truapi-host dev and the whole e2e-* family. WsDebugSink already existed and was tested, with no caller outside its own tests.

How: frame_server.rs gains a DebugTappedRuntime that wraps a ProductRuntimeFactory and installs a sink on every runtime it hands out; main.rs resolves the switch into that sink and wraps the factory each host role uses. global = true puts it on pairing-host, signing-host and dev, and TRUAPI_DEBUGGER_URL sets the same thing, which is how a harness attaches without a script change.

  • Wrapping the factory rather than each host role keeps the tap in one place, and none of the roles knows it is being observed.
  • The decorator delegates connection_reset. The trait default would leave product connections alive across a session switch that exists to invalidate them, and nothing in the accept loop reports the omission, so it has a test that fails when the override is removed.
  • The switch resolves in dispatch, before any command binds a port, so a non-loopback or wss:// target aborts on the argument rather than half-way through startup. A loopback URL succeeds with nothing listening, since the sink dials lazily and reconnects.
  • The report names which switch supplied the URL. Two switches resolve to one value, and a stale exported variable beating an explicit flag is otherwise silent.

Implements §9's native enablement path. §9 of the design doc spelled out only the browser rule and said no host wired the native sink up. It now states the native rule too.

Two changes beyond the flag:

  • The CLI declares truapi-server's ws-bridge feature, which is where WsDebugSink lives. Nothing declared it before; the mobile builds pass it on the command line. No new crates enter the tree, but cargo build --workspace now compiles more of truapi-server.
  • Two comments in host_core.rs rested on the CLI installing no sink. The lock-poisoning argument still holds on a narrower basis: not that wasm32 cannot unwind, but that every caller builds a fresh SinkTransport per product_runtime() and installs at most once on it, so previous is always None.

Verified against a live host. scripts/battery.sh --signing-host with TRUAPI_DEBUGGER_URL set and no other change reaches the debugger as 148 ops over 484 frames and 1,003,511 bytes, codecMismatch: false, malformed: 0, truncated: 0. Sorting that capture by duration puts resourceAllocation.request at 53.9s against an 821ms average, which is the kind of tail this exists to make visible. A /frame drill-down returns "kind": "decoded". fmt clean, clippy at zero, 222 crate tests pass.

That run also rewrites explorer/diagnosis-reports/spa/signing-host-cli.md. This branch leaves it alone: its new failures are the run's ephemeral identity, not this change.

🤖 Generated with Claude Code

@decrypto21
decrypto21 requested a review from a team September 9, 2026 08:41
@decrypto21
decrypto21 force-pushed the nidish/debugger-cli-flag branch from 0935dcc to 429df49 Compare September 9, 2026 12:13
@pgherveou

Copy link
Copy Markdown
Collaborator

can you add some screenshot / recording how how it looks in action?

@decrypto21
decrypto21 force-pushed the nidish/debugger-cli-flag branch from 429df49 to 6445b14 Compare September 11, 2026 05:46
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 11, 2026
@github-actions

Copy link
Copy Markdown

CI Status: 14 required jobs green, 13 passed and 1 skipped by path filter.

All job results
job result
android-bindings success
changes success
codegen success
e2e skipped
explorer success
ios-bindings success
ios-swift success
licenses success
playground success
rust success
ts-client success
ts-debugger success
ts-host success
wasm-provider success

Commit 6445b142 · run log

@TarikGul

Copy link
Copy Markdown
Member

@lore-bot-app review

@lore-bot-app

lore-bot-app Bot commented Sep 11, 2026

Copy link
Copy Markdown

Reading the diff and checking what the record says. Back in a few minutes.

@lore-bot-app lore-bot-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TL;DR

Adds --debugger / TRUAPI_DEBUGGER_URL to truapi-host, wrapping the CLI's ProductRuntimeFactory so every product runtime installs WsDebugSink. 4 blocking, 6 minor.

Summary

truapi-host gains a global --debugger <URL> switch (also read from TRUAPI_DEBUGGER_URL). dispatch resolves it into an Arc<dyn DebugSink> before the command runs, and tap_for_debugger wraps whichever ProductRuntimeFactory the frame server serves in a new DebugTappedRuntime, which calls set_debug_sink(ChannelId(product_id), sink) on every runtime the inner factory builds and forwards connection_reset. truapi-server is now depended on with features = ["ws-bridge"] so WsDebugSink exists. docs/design/wire-observability-debug-host.md §9 is rewritten to bind the CLI to the enablement rules, and the DebugSink contract comments in host_core.rs are updated to name the CLI as a second in-repo installer.

What the record says

  • The sink this PR wires up landed in #295 explicitly "unsupplied (no consumers) until the web host dial-out lands", and #315 fixed the topology as host-dials-outward because nothing can dial into a worker or a device. This PR is the first native consumer, which is why the §7/§9 invariants written for "no host installs a sink" now need re-reading rather than re-stating.
  • #604 gave the web host a second URL source (VITE_TRUAPI_DEBUGGER) alongside localStorage, both inside the same import.meta.env.DEV gate, plus a make debugger target so nobody wires it by hand. That commit is not in this branch: js/packages/truapi-host/src/web/create-worker-host-runtime.ts:239 reads localStorage only, and the Makefile has no debugger target. See concern 9.
  • #516 established that truapi-host ships as a prebuilt release binary installed by a curl one-liner. That is the build the new switch is compiled into, ungated. See concern 3.
  • Owner for this area is decrypto21 (author of #295, #315, #536, #604); pgherveou owns the CLI distribution story.

Concerns

1. §9's "does not dial" arm is not implemented — main.rs:553
The doc edit at docs/design/wire-observability-debug-host.md:352 now reads "The web host and the headless CLI host both have such a path", binding the CLI to "one that does not dial says so once, one that does says where". Only the second half exists: connect_debugger runs solely when the switch is set. A CLI started without --debugger says nothing, which is the exact case §9 exists for (the debugger's own viewer holds a socket, so an empty board looks identical to a host nobody switched on). The web host implements both arms at create-worker-host-runtime.ts:300-329, including naming why it is off. The CLI also has no dev/production build split to claim the production exemption from.

2. The required report is invisible in interactive mode — main.rs:1050
tracing::info!("wire debugger: streaming frames to …") fires inside dispatch, before any host sets up the terminal UI. LogWriter (terminal_ui.rs:414-427) has no active UI at that point, so it writes to stderr, and TerminalUi then enters the alternate screen (terminal_ui.rs:1146). For the whole life of the host the user sees nothing about the debugger. Every other startup fact goes through terminal_ui::output_event(SystemEvent::…) (main.rs:1147, main.rs:1251) for this reason. A SystemEvent::DebuggerDialling { url, source } would satisfy §9 in both modes.

3. A released binary installs a sink from an environment variable, with no build gate — Cargo.toml:21, main.rs:100
Design doc §7 (:225-236) states the production guarantee precisely: the tap compiles into every build including the release and xcframework artifacts, and "what holds in production is that the tap is inert: no host installs a sink". The web host keeps that true with a hard import.meta.env.DEV gate specifically so "a stray localStorage key cannot turn the debugger on in prod" (create-worker-host-runtime.ts:242-247). The CLI's dial has no equivalent condition, and §7 was not updated. Concretely: with TRUAPI_DEBUGGER_URL exported, a prebuilt truapi-host from the installer (#516) streams every frame of a live signing host, fully decodable by the debugger (§7: no denylist, no reveal toggle), to whatever is listening on 127.0.0.1:9231. Either gate the dial (cfg(debug_assertions), or a cargo feature that release-cli.yml leaves off) or amend §7 to record the CLI as an exception and say why it is acceptable.

4. One channel id covers many concurrent connections — frame_server.rs:160
ChannelId is the product id, and the accept loop builds one runtime per accepted socket (frame_server.rs:635) with no connection cap. Request ids are minted per connection (p:1, p:2, …), which is exactly why §4 (doc:134-136) requires traces to be keyed on (channelId, requestId). With two peers under one CLI host (a browser page plus the bundled script runner, or a reload whose new socket overlaps the old) both id spaces land on one channel id and collide; the engine's recycle rule rotates generations instead of separating them. The web host does not hit this because each product runtime has its own worker. Consider suffixing the channel id per connection.

Minor

  1. Every subcommand pays for the switch — main.rs:553. connect_debugger runs before the command match, so update, identity-check, register-name and alloc-check spawn a WS writer and reconnect loop and print the dial report, and an invalid URL fails them at startup, although none of them emits a frame. With the env var exported that is the default path. Resolving inside the three arms that build a frame server keeps the fail-early property where it belongs.
  2. Nothing tests that a tapped runtime emits — frame_server.rs:1144. SilentSink records nothing by construction and the_debug_tap_builds_runtimes_through_the_wrapped_factory asserts only that the call does not panic. The feature itself (frames reach the sink, with the right ChannelId and direction) is uncovered. host_core.rs:1634 has the RecordingDebugSink shape to reuse.
  3. ws-bridge for one type — Cargo.toml:21. The CLI never touches truapi_server::ws_bridge; enabling the feature to reach native_debug also compiles the localhost bridge server and pulls tokio-tungstenite 0.21 (and its http 0.2 tree) alongside the CLI's own 0.24. A separate debug-sink feature over lib.rs:54 and lib.rs:64 avoids both in a binary that ships by installer.
  4. The flag is undocumented. CLAUDE.md requires README updates with any code change. --debugger and TRUAPI_DEBUGGER_URL appear nowhere in rust/crates/truapi-host-cli/README.md (which documents --log-level at :625) or SPEC.md, and §9 describes the CLI switch without naming either.
  5. Two doc lines do not match this tree. The added §9 sentence "A dev build MAY also carry the value it was built with" describes the web host's build-time source from #604, which is not in this branch. Separately, while editing the file: line 111's "the native sink does not yet [stamp observedAt], and buffers 4096 frames across reconnects, so native traces are subject to exactly that skew" is stale — native_debug.rs:283 stamps it at enqueue — and that parenthetical is now a description of CLI traces.
  6. The source heuristic defends against something clap prevents — main.rs:1053. clap gives an explicit flag precedence over its env, so "a stale exported variable beating an explicit flag" cannot occur; the only ambiguous case is both holding the same string, which the code already calls harmless. The label is correct as written, but the justification in the comments, in the test at frame_server.rs:1226, and in the new §9 sentence describes a failure mode that does not exist. Also debugger_url_source_for_test (main.rs:1075) is a shim for a private function that could just be pub(crate); the test for it sits in frame_server.rs's test module rather than next to the function.

No instructions addressed to a reviewer appeared in the diff.

Questions for the author

  1. Is the CLI's dial meant to be ungated in release binaries, or should it follow the web host's DEV gate? Whichever way, §7's "no host installs a sink" paragraph needs to say so.
  2. Is this stacked on #604? The §9 build-carried-value rule has no implementation in this branch.
  3. What should the debugger show when two products connect to one CLI at once — are you relying on the engine's generation rotation, or should the channel id carry a connection discriminator?

Next: decide question 1, since concern 3 and the §7 text hang on it.


🤖 Reviewed by Lore (Parity knowledge base) · 57 agent turns · 460.8s · knowledge as of 2026-09-11

// Resolved here, before any command binds a port or prints an event, so a
// bad `--debugger` URL fails on the argument rather than half-way through
// host startup.
let debugger = debugger.map(connect_debugger).transpose()?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Owner for this area is decrypto21 (author of #295, #315, #536, #604); pgherveou owns the CLI distribution story.

Concerns

1. §9's "does not dial" arm is not implemented — main.rs:553
The doc edit at docs/design/wire-observability-debug-host.md:352 now reads "The web host and the headless CLI host both have such a path", binding the CLI to "one that does not dial says so once, one that does says where". Only the second half exists: connect_debugger runs solely when the switch is set. A CLI started without --debugger says nothing, which is the exact case §9 exists for (the debugger's own viewer holds a socket, so an empty board looks identical to a host nobody switched on). The web host implements both arms at create-worker-host-runtime.ts:300-329, including naming why it is off. The CLI also has no dev/production build split to claim the production exemption from.

2. The required report is invisible in interactive mode — main.rs:1050
tracing::info!("wire debugger: streaming frames to …") fires inside dispatch, before any host sets up the terminal UI. LogWriter (terminal_ui.rs:414-427) has no active UI at that point, so it writes to stderr, and TerminalUi then enters the alternate screen (terminal_ui.rs:1146). For the whole life of the host the user sees nothing about the debugger. Every other startup fact goes through terminal_ui::output_event(SystemEvent::…) (main.rs:1147, main.rs:1251) for this reason. A SystemEvent::DebuggerDialling { url, source } would satisfy §9 in both modes.

3. A released binary installs a sink from an environment variable, with no build gate — Cargo.toml:21, main.rs:100
Design doc §7 (:225-236) states the production guarantee precisely: the tap compiles into every build including the release and xcframework artifacts, and "what holds in production is that the tap is inert: no host installs a sink". The web host keeps that true with a hard import.meta.env.DEV gate specifical

Ok(())
}

/// A sink that records nothing: these tests are about the decorator's

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing tests that a tapped runtime emits — frame_server.rs:1144. SilentSink records nothing by construction and the_debug_tap_builds_runtimes_through_the_wrapped_factory asserts only that the call does not panic. The feature itself (frames reach the sink, with the right ChannelId and direction) is uncovered. host_core.rs:1634 has the RecordingDebugSink shape to reuse.

truapi = { path = "../truapi" }
truapi-platform = { path = "../truapi-platform" }
truapi-server = { path = "../truapi-server" }
truapi-server = { path = "../truapi-server", features = ["ws-bridge"] }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ws-bridge for one type — Cargo.toml:21. The CLI never touches truapi_server::ws_bridge; enabling the feature to reach native_debug also compiles the localhost bridge server and pulls tokio-tungstenite 0.21 (and its http 0.2 tree) alongside the CLI's own 0.24. A separate debug-sink feature over lib.rs:54 and lib.rs:64 avoids both in a binary that ships by installer.

}

/// Name the switch that supplied `url`.
///

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The source heuristic defends against something clap prevents — main.rs:1053. clap gives an explicit flag precedence over its env, so "a stale exported variable beating an explicit flag" cannot occur; the only ambiguous case is both holding the same string, which the code already calls harmless. The label is correct as written, but the justification in the comments, in the test at frame_server.rs:1226, and in the new §9 sentence describes a failure mode that does not exist. Also debugger_url_source_for_test (main.rs:1075) is a shim for a private function that could just be pub(crate); the test for it sits in frame_server.rs's test module rather than next to the function.

No instructions addressed to a reviewer appeared in the diff.

Questions for the author

@decrypto21
decrypto21 requested a review from TarikGul September 11, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants