Skip to content

Emit a neutral daemon:data-plane-ready event and reload on it instead of the capability-probe event - #3417

Merged
mstomar125 merged 3 commits into
mainfrom
issue-3318-data-plane-ready-event
Oct 1, 2026
Merged

mstomar125 merged 3 commits into
mainfrom
issue-3318-data-plane-ready-event

Conversation

@mstomar125

@mstomar125 mstomar125 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Part of #3412

What

The free app reloaded schemas and collections on the Pro capability probe's event, which only worked because that probe happened to be the first real gRPC round trip at startup. Core now emits its own neutral event once the daemon answers, and the shell listens to that. This covers #3412's "any reload core still needs after the probe goes is driven by a core-named event" item; the Rust probe, its emit and ProClient::rebind stay for a later #3412 slice to delete.

  • services/grpc_client.rs: GrpcClient::data_plane_round_trip(timeout) -> DataPlaneRoundTrip sends one GetNode for the sentinel id __ns_channel_probe__ on the shared channel, bounded by the timeout. It returns:

    • Answered for a response or NotFound, which is what a healthy daemon returns for that id;
    • Failed(code) for any other completion;
    • TimedOut when nothing completes in time.

    It also adds DATA_PLANE_PROBE_TIMEOUT (5 s). The enum and the constant are re-exported from services.

  • commands/nodes.rs probe_and_recover: the private probe(), PROBE_TIMEOUT and PROBE_ID are gone. Both probes are now !matches!(client.data_plane_round_trip(DATA_PLANE_PROBE_TIMEOUT).await, DataPlaneRoundTrip::TimedOut), so any completion still counts as live and only a timeout counts as a wedge. The rebind block and its doc paragraph are untouched.

  • lib.rs:

    • Adds pub(crate) const DATA_PLANE_READY_EVENT = "daemon:data-plane-ready".
    • The startup task clones the client before the token stream takes it.
    • The reachability block now evaluates to socket_reachable. Its not_running emit and its nodespaced unreachable after startup log line are byte-identical.
    • After that block, when the socket is reachable and the round trip is Answered, the task broadcasts the event with a null payload (info! on success, warn! if the emit fails). Otherwise it logs a warn! with the outcome and emits nothing.
    • The reachability comment no longer refers to the probe.
    • The Pro probe block and the daemon-ready spawn are unchanged.
  • daemon-status.ts: exports DATA_PLANE_READY_EVENT, mirroring the Rust constant. The module doc names the neutral event and no Pro event.

  • app-shell.svelte: unlistenTier is renamed unlistenDataPlaneReady, along with its cleanup, and listens on DATA_PLANE_READY_EVENT with the same two reload calls. The schemasData import and the proSync lines are left for the [Core release blocker] Strip all Pro code, edition branching and sync/ACL state from core in one pass (ADR-081 steps 0 and 2) #3412 slice that strips the Pro frontend.

  • props-driven-components.test.ts: the tautological test is replaced by source-contract tests (details below).

The event is core-internal

daemon:data-plane-ready is not part of the versioned extension API (ADR-082 §3.5, §8):

  • The Rust constant is now pub(crate). Only the startup task and its inline tests use it; the TS contract test reads the Rust source text, not the symbol.
  • Its Rust doc and the DATA_PLANE_READY_EVENT export doc in daemon-status.ts both say so, and point extensions at the host API's onDaemonReconnect, which also fires on the session's first healthy status.
  • It is not re-exported from src/lib/extension-api. The extension-api surface snapshot is unchanged and EXTENSION_API_VERSION stays 1.0.

Deviations from the original plan

  • The source-text test lives in the existing run_wiring_tests module, not a new data_plane_ready_emit_tests. That module already has run_source() covering run and the app build it drives. The new test run_emits_data_plane_ready_after_the_reachability_check_once_the_daemon_answers slices from nodespaced unreachable after startup to StreamingTaskRegistry::default(). It asserts that data_plane_round_trip( comes before emit(DATA_PLANE_READY_EVENT, and that run contains the emit exactly once. run_spawns_daemon_ready_tasks_once_the_watcher_and_token_stream_are_wired is unchanged and still passes: the new block sits after the reachability line, and the spawn sits before it.
  • probe_and_recover's existing source-text test (..._between_the_rebuild_and_the_reprobe) found the re-probe by rfind("probe("). The private probe() is deleted, so that needle now finds the re-probe by rfind("data_plane_round_trip(").
  • Paths follow the crate split: packages/desktop-app/app-lib and -p nodespace-app-lib, not src-tauri and -p nodespace-app.
  • BASELINES are not lowered. Parallel [Core release blocker] Strip all Pro code, edition branching and sync/ACL state from core in one pass (ADR-081 steps 0 and 2) #3412 PRs collide on those lines, and the [Core release blocker] Strip all Pro code, edition branching and sync/ACL state from core in one pass (ADR-081 steps 0 and 2) #3412 slice that rewrites the checker sets them all.

Pro-boundary ratchet

bun run scripts/check-pro-boundary.ts passes, and no count is above its baseline. Delta against origin/main (247e0dea), by marker:

  • proEvents 81 → 77 (−4):
    • app-shell's listen('pro:tier-detected', ...);
    • the daemon-status.ts module doc;
    • the old test's two lines.
  • proWording 235 → 233 (−2): app-shell's two comment lines.

Every other marker is unchanged. No marker line is added. No BASELINES change (see above).

Tests

Each new test was mutation-checked: I broke the code path it covers, confirmed the test fails, then restored the code.

Test Kind Mutation Result
a_refused_endpoint_fails_without_timing_out unit, grpc_client.rs non-NotFound failure reported as Answered fails
a_silent_endpoint_times_out unit, grpc_client.rs timeout reported as Failed(DeadlineExceeded) fails
run_emits_data_plane_ready_after_the_reachability_check_once_the_daemon_answers source-text, lib.rs extra emit before the round trip, ungated fails ("emitted only after the daemon answers")
probe_and_recover_runs_the_channel_rebuilt_hooks_between_the_rebuild_and_the_reprobe (needle updated) source-text, nodes.rs hooks moved after the re-probe fails
the_data_plane_round_trip_answers_once_the_daemon_is_healthy integration, real spawned daemon NotFound arm removed fails (left: Failed(NotFound), right: Answered)
reloads schemas and collections on the data-plane-ready event frontend contract listener pointed at another event / back at the legacy event / loadCollections() dropped fails each time
no longer listens for the legacy event frontend contract listener pointed back at the legacy event fails
names the same event the Rust startup task emits frontend contract TS constant drifted from Rust fails
NavigationSidebar does not import the Tauri event API frontend contract sidebar imports @tauri-apps/api/event fails

Mutation checks for this pass, on the rebased tree:

Test Mutation Result
run_emits_data_plane_ready_after_the_reachability_check_once_the_daemon_answers (now also pins the Answered gate between the round trip and the emit) gate loosened to emit on any completion (outcome.is_some() && outcome != Some(DataPlaneRoundTrip::TimedOut)), run through bun run test:changed fails (306 passed, 1 failed: this test)
names the same event the Rust startup task emits (sources now read from __dirname) Rust literal drifted fails
NavigationSidebar does not import the Tauri event API (sources now read from __dirname) sidebar imports @tauri-apps/api/event fails

Runs on origin/main @ 247e0dea, head 6f26e839:

  • cargo fmt -p nodespace-app-lib -- --check and cargo clippy -p nodespace-app-lib --all-targets -- -D warnings: clean. --all-targets compiles tests/it against the pub(crate) constant.
  • bun run quality:fix: clean, with no further changes.
  • bun run --cwd packages/desktop-app test src/tests/extension-api: 43 passed, and extension-api-surface.json is unchanged.
  • bun run test:changed: passed. Tiers: frontend Happy-DOM, 5650 tests in 311 files; scripts; browser; Rust workspace nextest, 4149 passed; app-lib units, 307 passed. Skill was out of scope.
  • app-lib's tests/it binary (the_data_plane_round_trip_answers_once_the_daemon_is_healthy) is not in test:changed's Rust tier. The merge gate's Tauri-seam stage runs it. Its code is unchanged since the earlier run on 86b7190e.

Earlier runs (on 86b7190e, then a7d2f93d):

  • cargo test --lib -p nodespace-app-lib -- data_plane run_wiring_tests probe_and_recover: 12 passed.
  • nextest -p nodespace-app-lib --test it startup_readiness_data_plane_test:: extension_hooks_test::: 4 passed. This includes a_wedged_channel_rebuild_runs_the_hooks_before_recovery_returns, so a timeout still triggers a rebuild through the shared method.

Not done

  • Manual bun run dev:tauri GUI check. The machine is shared, and that check needs a full app build and a GUI window. The integration test exercises the round trip against a real spawned daemon, and the source-text test pins the emit's placement.

🤖 Generated with Claude Code

@mstomar125

Copy link
Copy Markdown
Contributor Author

Code review: pragmatic-code-reviewer, medium depth

Reviewed at head 48fd7a6d (rebased on origin/main @ 247e0dea), covering the full PR diff. Verdict: APPROVE.

Code Review Summary

This PR is a net improvement and is scoped correctly. Core now owns a neutral readiness signal, daemon:data-plane-ready. It is emitted once at startup, after the reachability check, and only when a real GetNode round trip comes back Answered. App-shell stops depending on the legacy capability-probe event, and the channel probe in probe_and_recover is now a shared helper with a small typed result enum.

What I checked in the code:

  • probe_and_recover behaviour is unchanged. Only TimedOut triggers the rebuild, the same as the old "any completion means alive" check. The existing seam test still anchors the re-probe after run_channel_rebuilt_hooks.
  • The daemon's NotFound counts as an answer. get_node → fetch_node returns Status::not_found for an unknown id (packages/daemon/src/services/node_service.rs). Mapping that to Answered is right, and the integration test checks it against a real daemon.
  • Broadcasting with app_handle.emit is the right choice. window_routing::emit_routed(.., None) would reach only the focused window once windows are pinned. Every window needs the reload, and the old event broadcast too.
  • Only onDaemonReconnect is re-exported. src/lib/extension-api/index.ts re-exports it by name, so DATA_PLANE_READY_EVENT does not leak into the versioned extension API. The ADR-082 §3.5 and §8 citations match the ADR text. The claim that onDaemonReconnect "fires on the first healthy status" matches daemon-status.ts.
  • The NavigationSidebar test is better. The old version was a tautology; it now checks the real file contains no @tauri-apps/api/event import.
  • Repo rules hold. The legacy-event needle is built from fragments. There are no issue numbers or Pro wording in new code or comments, and no lint suppressions.

Findings

Critical Issues

None.

Suggested Improvements

[Improvement] The new wiring test doesn't check that the emit only happens on Answered. run_emits_data_plane_ready_after_the_reachability_check_once_the_daemon_answers checks only that data_plane_round_trip( comes before emit(DATA_PLANE_READY_EVENT and that the emit appears exactly once. Changing if outcome == Some(DataPlaneRoundTrip::Answered) to if outcome.is_some() would pass every test in the PR, yet the test's name and message claim exactly that property. Fix: also find the gate in tail and assert round_trip < gate < emit.

Nitpicks

  • Nit: socket_reachable is also true when the daemon status is Starting; only NotRunning maps to false. Behaviour is fine (the round trip then fails or times out, so nothing is emitted), but the name overstates it.
  • Nit: On the unreachable path, the else branch logs warn!(?outcome /* None */, "… not answering …; not emitting") right after the error!("nodespaced unreachable after startup"). The round trip was skipped, not left unanswered.
  • Nit: props-driven-components.test.ts reads files relative to the cwd, with an inaccurate "see its vitest config root" comment. Sibling tests resolve from __dirname (typed-core-fields.test.ts, adapter-core.test.ts).

APPROVE

🤖 Generated with Claude Code

@mstomar125

Copy link
Copy Markdown
Contributor Author

Address-review

Fixes are in 6f26e839. The same reviewer re-reviewed that commit: no further findings, APPROVE.

Finding Decision What changed
[Improvement] The wiring test doesn't pin the Answered gate Accepted run_emits_data_plane_ready_... also finds if outcome == Some(DataPlaneRoundTrip::Answered) and asserts round trip < gate < emit. The test cannot match its own needle, because run_source() covers only run.
Nit: test sources read relative to the cwd Accepted A module-level readDesktopAppFile resolves from __dirname (../../.. is packages/desktop-app). It replaces both cwd-relative reads and the inaccurate comment.
Nit: socket_reachable is true on Starting Declined The comment just above the block already says "Starting is transient — only NotRunning trips the banner". The emit guarantee rests on the Answered gate, which is now pinned. This slice changes nothing else in the design.
Nit: warn! with outcome = None after the unreachable error Declined Only the log wording is affected, and behaviour is correct. Same scope reason.

The if socket_reachable guard is deliberately left unpinned. Dropping it costs only one bounded round trip against a down daemon, and it cannot cause an emit, because the Answered gate still decides.

Mutation checks

  • Gate loosened to emit on any completion (outcome.is_some() && outcome != Some(DataPlaneRoundTrip::TimedOut)), run through bun run test:changed: exactly that test fails (306 passed, 1 failed). Restored.
  • Rust event literal drifted: "names the same event the Rust startup task emits" fails. Restored.
  • The sidebar imports @tauri-apps/api/event: the sidebar test fails. Restored.

Runs on 6f26e839

  • bun run quality:fix: clean.
  • bun run test:changed: passed. That covers the frontend (5650 tests), scripts, browser, Rust workspace nextest (4149 tests) and app-lib units (307 tests).
  • The pre-push ratchet passes.

No re-review is needed beyond the one already done: the change is test-only, and the reviewer approved it.

🤖 Generated with Claude Code

@mstomar125
mstomar125 marked this pull request as ready for review October 1, 2026 09:59
mstomar125 and others added 3 commits October 1, 2026 12:05
…f pro:tier-detected

- GrpcClient::data_plane_round_trip sends one bounded GetNode for a sentinel id
  and reports Answered (a response or NotFound), Failed(code) or TimedOut.
- probe_and_recover uses it, keeping "any completion is live, only a timeout
  is a wedge".
- The startup task emits DATA_PLANE_READY_EVENT once, after the reachability
  check, when the socket is reachable and the daemon answers the round trip.
- app-shell reloads schemas and collections on that event; the tautological
  test is replaced with source-contract tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ernal

- lib.rs: DATA_PLANE_READY_EVENT becomes pub(crate); only the startup task and
  its inline tests use it. Its doc states the event is not part of the
  versioned extension API (ADR-082 §3.5, §8) and points extensions at
  onDaemonReconnect.
- daemon-status.ts: the DATA_PLANE_READY_EVENT export doc carries the same
  note and that it is not re-exported from $lib/extension-api.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st sources from __dirname

- lib.rs run_wiring_tests: the emit test also finds the
  `outcome == Some(DataPlaneRoundTrip::Answered)` gate and asserts
  round trip < gate < emit, so emitting on any completion fails it.
- props-driven-components.test.ts: the source-contract tests read files
  through readDesktopAppFile, resolved from __dirname instead of the runner's
  cwd, replacing the inaccurate "vitest config root" comment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant