iter-137: daemon-mode parity — frame targets, RPC queueing, honest timeouts - #176
Merged
Conversation
…nned network source
Everything iteration 129 shipped worked only with --no-daemon. In the default
connection mode frame enumeration returned zero targets, so `click --frame`,
the cross-origin frame scan and `consent accept` all silently degraded. Every
iter-129 live test passed --no-daemon, so nothing caught it.
Theme A — frame-target enumeration through the daemon proxy.
The plan's stated root cause (event sink installed too late) was wrong. Two
real causes, both confirmed on the wire against Firefox 153:
1. `watchTargets` is not repeatable on a connection.
ParentProcessWatcherRegistry.watchTargets only adds the target type to
the watcher's session data, so the daemon — which subscribes once at
startup — makes every proxied client's watchTargets("frame") a no-op and
the drain window empty by construction. Without
isServerTargetSwitchingEnabled: true the daemon also received no
target-available-form at all (`daemon status` reported target_count: 0).
2. `navigate` was tearing the daemon's subscription down. Its three
unwatchTargets("frame") teardown calls landed on the shared connection,
and under server-side target switching unwatchTargets destroys every
target, top level included: two available forms followed by four
destroyed forms after each navigation.
Fix: the daemon requests its watcher with server-side target switching on,
records every raw target form in SharedState::frame_targets (deduped by actor
id, removed on destroy, cleared on a real top-level switch), serves it over a
new {"to":"daemon","type":"frame-targets"} request, and drops client
unwatchTargets frames (is_client_target_teardown — safe, the method is oneway).
The CLI replays the packets through the new
ff_rdp_core::target_events_from_packets, the same add/replace/remove rules the
direct drain applies, from one shared entry point
(commands/frame_targets.rs::fetch_frame_targets) that click and consent both
use. `daemon status` gains live_target_count and the frame-targets reply
carries watcher_ready, so an unestablished subscription reports
daemon_watcher_not_ready instead of "0 frames".
Theme B — concurrency. claim_rpc_slot_queued replaces iter-101's instant
refusal with a Condvar queue (RPC_QUEUE_BUDGET, 2s daemon-queued keep-alives
so a waiting client's socket read window keeps restarting); daemon_busy now
reports waited_ms and names the cap. AppError::from(ProtocolError::Timeout)
reports the socket read deadline instead of a fabricated after_ms: 0, and
Display never renders a zero as an elapsed duration.
Theme C — `network --source auto|watcher|performance-api` pins the capture
source so both connection modes return the same rows; meta.source_reason is
always present and states which rule applied. --since with performance-api is
refused (since_requires_watcher_source) rather than silently ignored.
Theme D — tests/no_daemon_live_test_guard.rs fails any live suite that uses
--no-daemon without a `//! daemon-parity:` declaration, with a shrink-only
grandfather list. It caught this iteration's own new suite.
Verified live on Firefox 153: iteration-129's dogfood_path passes as written
(consent accept -> {"cmp":"sourcepoint","action":"accepted"} without
--no-daemon, click --frame lists 3 frames, 4/4 concurrent page-text). Full CLI
live suite 170/170 and core live suite green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All acceptance criteria landed and are verified against the diff (ac-fidelity-check passes); no code changes needed from /review-pr — the daemon-mode-parity implementation held up under a full local review of the RPC-queueing, frame-target snapshotting, and network source-selection logic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Everything iteration 129 shipped worked only with
--no-daemon. In the default connection mode frame enumeration returned zero targets, soclick --frame, the cross-origin frame scan andconsent acceptall silently degraded. It went green because every iter-129 live test passed--no-daemon— the tests and the iteration's owndogfood_pathdisagreed, and the tests won.Theme A — frame targets through the daemon proxy. The plan's stated root cause (an event sink installed too late) was wrong; two real causes, both confirmed on the wire against Firefox 153:
watchTargetsis not repeatable on a connection —ParentProcessWatcherRegistry.watchTargetsonly adds the target type to the watcher's session data, so the daemon (which subscribes once at startup) makes every proxied client'swatchTargets("frame")a no-op and its drain window empty by construction. WithoutisServerTargetSwitchingEnabled: truethe daemon received notarget-available-format all (daemon statusreportedtarget_count: 0for whole sessions).navigatewas tearing the daemon's subscription down — its threeunwatchTargets("frame")teardown calls landed on the shared connection, and under server-side target switchingunwatchTargetsdestroys every target, top level included. Captured in the daemon log: 2target-available-formimmediately followed by 4target-destroyed-formafter each navigation.Fix: the daemon records every raw target form (
SharedState::frame_targets), serves it over a new{"to":"daemon","type":"frame-targets"}request, and drops clientunwatchTargetsframes (is_client_target_teardown— safe, the method isoneway). The CLI replays the packets through the newff_rdp_core::target_events_from_packets(the same add/replace/remove rules the direct drain uses) from one shared entry point,commands/frame_targets.rs::fetch_frame_targets, thatclickandconsentboth call.Theme B — concurrency.
claim_rpc_slot_queuedreplaces iter-101's instant refusal with aCondvarqueue (RPC_QUEUE_BUDGET, 2 sdaemon-queuedkeep-alives so a waiting client's socket read window keeps restarting);daemon_busynow reportswaited_msand names the cap.AppError::from(ProtocolError::Timeout)reports the socket read deadline instead of a fabricatedafter_ms: 0, andDisplaynever renders a zero as an elapsed duration.Theme C — network source. New
network --source auto|watcher|performance-apipins the capture source so both connection modes return the same rows;meta.source_reasonis always present and states which rule applied.--sincewithperformance-apiis refused (since_requires_watcher_source) rather than silently ignored.Theme D — the discipline hole.
tests/no_daemon_live_test_guard.rsfails any live suite that uses--no-daemonwithout a//! daemon-parity:declaration, with a shrink-only grandfather list. It caught this iteration's own new suite before it was annotated.Observability added along the way:
daemon statusgainslive_target_count(targets alive now, vs. the cumulativetarget_count— the two diverging is this bug's signature), and theframe-targetsreply carrieswatcher_readyso an unestablished subscription returnsdaemon_watcher_not_readyinstead of an empty snapshot presented as fact.Test plan
cargo fmt,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace -q— all cleancargo run -p xtask -- check-iteration-ready --plan kb/iterations/iteration-137-daemon-mode-parity.md --base origin/main— 10/10 PASS (withFF_RDP_LIVE_TESTS=1);check-daemon-locksandcheck-oneway-conformancealso passlive_137_frame_targets_via_daemon— 2 frames via daemon and via--no-daemon, equallive_137_click_cross_origin_via_daemon—tag: "A",meta.frame_url: "https://example.com/", no--no-daemonlive_137_concurrent_commands— 4/4 concurrent proxied commands succeed, no0msduration in any outputlive_137_network_source_parity—meta.source,meta.source_reasonand row count identical in both modes with--source performance-apilive_137_consent_accept_via_daemon(network-gated) — verified manually against theguardian.com:{"cmp":"sourcepoint","action":"accepted"}immediately afternavigate, without--no-daemondogfood_pathre-run verbatim:consent accept→{"cmp":"sourcepoint","action":"accepted"},click --frame→ 3 frames listed (was0 frame(s) available:), 4/4 concurrentpage-textFF_RDP_LIVE_TESTS=1 cargo test -p ff-rdp-cli --test live -- --include-ignored --test-threads=1— 170 passed, 0 failed (regression check for enabling server-side target switching on the daemon)FF_RDP_LIVE_TESTS=1 cargo test -p ff-rdp-core -- --include-ignored --test-threads=1— green (running it in parallel shows pre-existing cross-test interference, unrelated to this branch)Spec drift
None. The only
crates/ff-rdp-core/src/actors/*.rschange istarget_events_from_packets, a pure function over packets already received — it issues no RDP requests and introduces no undeclared spec fields.🤖 Generated with Claude Code## Claims vs code
<generated 2026-08-09T22:08:22Z by ralph-loop>
the→ ✅ matched in diffSharedState::frame_targets→ ✅ matched in diffAppError::from→ ✅ matched in diffProtocolError::Timeout→ ✅ matched in diffRPC_QUEUE_BUDGET→ ✅ matched in diffframe-target→ ✅ matched in difftarget-available-form→ ✅ matched in diffframe-targets→ ✅ matched in diff