communitas: harden x0x app integration

[dirvine]

Summary

• harden Swift and Dioxus x0x live receive paths with reconnect/re-subscribe and macOS receive defaults
• make /direct/send require_gossip_ack intent explicit for UI DMs
• add real Swift and Dioxus publish-to-WebSocket-receive live messaging coverage
• point the release signer pin and live messaging workflow at x0x v0.22.0

Validation

• cargo fmt --all
• cargo clippy --all-features --all-targets -- -D warnings -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used
• cargo check --workspace --all-targets
• cargo test -p communitas-x0x-client --test client_coverage
• cargo test --manifest-path communitas-dioxus/tests/e2e/Cargo.toml --test e2e_messaging --no-run
• swift test
• X0X_LIVE_TESTS=1 X0XD_BIN=/Users/davidirvine/Desktop/Devel/projects/x0x/target/release/x0xd swift test --filter PubSubRoundTripTests

Known blocker

As of this PR creation, saorsa-labs/x0x still shows PR #100 open/draft, latest release v0.21.3, and no visible v0.22.0 tag. The communitas workflow intentionally defaults to v0.22.0 so the release-state mismatch is visible in CI until the tag/release exists.

[claude]

Code Review: communitas: harden x0x app integration

Overview

This PR hardens the WebSocket live-receive paths in both the Dioxus (Rust) and Swift macOS apps by replacing single-connect patterns with exponential-backoff reconnect loops (1s initial, 2× multiplier, 30s cap) that re-subscribe topics on every reconnect. It makes require_gossip_ack explicit for all DM sends, adds real live E2E messaging tests for both apps, pins CI to x0x v0.22.0, replaces SwiftUI components that caused macOS layout crashes with NSViewRepresentable equivalents, and upgrades Dioxus from 0.7.3 to 0.7.9.

The reconnect pattern itself is solid and consistently applied across 7+ WS connection sites. The review surfaces three blocking issues that should be resolved before merge, plus several non-blocking items.

---

🔴 Blocking Issues

1. rav1e / libfuzzer-sys transitive dependency via Dioxus 0.7.9

dioxus 0.7.9 pulls image → ravif → rav1e → libfuzzer-sys. Per CLAUDE.md: "cargo build --all-targets fails due to libfuzzer-sys (Linux-only)." This will break Windows CI and bloat the binary with an AV1 video encoder that serves no purpose in a chat app.

Fix: feature-gate the image crate in communitas-dioxus/Cargo.toml:

image = { version = "0.25", default-features = false, features = ["png", "jpeg", "gif", "webp"] }

2. WebSocket Bearer token leaked into URL query string

In communitas-x0x-client/src/websocket.rs, websocket_url_from_env() appends the token as:

format!("{url}?token={}", token.trim())

Tokens in URL query strings appear in server access logs, proxy logs, and browser history. For a security-focused PQC platform this is a policy violation.

Fix: Send the token as an Authorization: Bearer <token> header on the HTTP upgrade request using tokio-tungstenite's connect_async_with_config with a custom Request.

3. GroupPublicSendResponse missing ok: bool field (Rust)

In communitas-x0x-client/src/types.rs, GroupPublicSendResponse has no ok: bool field, but the x0x 0.22 API and the Swift GroupPublicSendResponse model both include it. Any send returning ok: false will be silently treated as success by the Rust client since it only checks HTTP status.

Fix: Add ok: bool to the Rust type to match the API contract.

---

🟡 Non-Blocking Bugs

• waitForFrame() timeout unbounded (PubSubRoundTripTests.swift): The outer while loop checks Date() < deadline but passes the original full timeoutSeconds to each receiveFrame() call. If non-matching frames arrive slowly, total elapsed time can far exceed timeoutSeconds, causing silent CI hangs. Track remaining deadline time per iteration:

  let remaining = max(1, Int64(deadline.timeIntervalSinceNow))
  let frame = try await receiveFrame(ws, timeoutSeconds: UInt64(remaining))

• createPage() loses user input on failure (WebPublishView.swift, WikiView.swift): The text field is cleared before the async network call completes. On failure, the typed path is permanently lost. Keep the field populated until the Task succeeds.

• ChannelManager.openWebSocketAndResubscribe(): resubscribeActiveTopics() is called before the WS is returned to the caller. If resubscription fails internally (it swallows errors and sets errorMessage), the receive loop starts with a silently missing subscription — messages will not be delivered.

• DirectMessageView.startListening(contact:): connectToAgent(contact:force:true) is called on every reconnect iteration. Repeated connect-agent calls may be rate-limited or rejected (409 conflict) by the daemon, causing constant error noise. Add per-contact debounce or separate backoff for the connect step.

• ChannelManager.resubscribeActiveTopics(): Derives topics from currentChannel at reconnect time. If navigation occurred during the outage, the re-subscribed topic set may not match what was active before disconnect. Snapshot active topics at subscribe time and replay the exact set on reconnect.

---

🟡 Code Quality

• set_signal_if_changed<T: PartialEq + 'static>() is copy-pasted in at least 4 Dioxus modules. Extract to communitas-dioxus/src/signal_utils.rs.

• positionMainWindowForAutomationIfRequested() schedules 12 DispatchQueue.main.asyncAfter closures unconditionally and never short-circuits after the first successful window position. All 12 timers always fire, and with 4+ call sites, up to 48 timers can be in-flight at startup. Replace with a self-cancelling recursive helper.

• polling_timers_enabled_for_env(_is_macos: bool, _enable_polls: bool): Both parameters are prefixed with _ (explicitly unused). The function ignores _enable_polls and returns !disable_polls regardless. Either remove the dead parameter or implement the intended macOS opt-in logic.

• NetworkView.pollData(): All per-call errors are swallowed with try?, losing diagnostic signal the old code provided.

• scheduleBoardStartup / scheduleFeedStartup / scheduleSwarmStartup: Use DispatchQueue.main.async { Task { @MainActor in ... } } throughout. The outer DispatchQueue.main.async is redundant — Task { @MainActor in ... } already hops to the main actor.

• NetworkView.diagnostics(for:) makes sequential peerHealth + probePeer calls for up to 12 peers every 5-second poll cycle (~24 sequential async calls). Use async let or withTaskGroup to parallelize across peers.

---

🟡 Performance

• Full message list fetch on every send (channel_chat.rs): get_group_public_messages() is called after every successful send to refresh the full history. This is O(message_count) network work per send and will cause visible lag in active channels. Append the sent message optimistically and let the WS receive loop drive subsequent updates.

• AppShell polling loop (communitas-dioxus/src/main.rs): client.presence().await and client.discovered_agents().await are called sequentially every 8 seconds. These are independent — use join!().

---

✅ What's Done Well

• Reconnect/backoff pattern is consistent and correct across all 7+ WS connection sites: 1s initial, 2× multiplier, 30s cap, reset on successful connect.
• clearWebSocket(_ ws:) uses identity check (===) before nilling, correctly preventing a race where a newer socket gets cleared by an older task's error handler.
• X0xWebSocket Drop impl aborts both send_task and receive_task JoinHandles — no orphaned background tasks on drop.
• path_component() in the Rust client correctly percent-encodes all non-unreserved bytes, applied consistently to all store/task path segments. Good path traversal prevention.
• require_gossip_ack = true is now the default for all DM sends — DMs surface failure instead of silently dropping.
• AppKitFormControls.swift (NSViewRepresentable components) is the correct long-term fix for macOS AutoLayout constraint crashes. Coordinator pattern with weak references is implemented correctly.
• AppShell polling loop uses peek() before set() for all signals, avoiding spurious re-renders on unchanged data.
• DaemonManager health probe timeout (5s) prevents the onboarding check from blocking indefinitely.
• key: '{space_id}:{tab}' in SpaceView RSX ensures Dioxus re-mounts the component (and its WS coroutines) on navigation between spaces — prevents stale channel state.
• Real live E2E round-trip tests for both Swift (PubSubRoundTripTests) and Dioxus (messaging_pubsub_roundtrip) are well-structured and correctly gated behind DaemonFixture.liveTestsEnabled / #[ignore].
• SecureDecryptResponse gains #[serde(alias = "payload_b64")] for forward-compatible decoding across API versions.
• is_running_health_status() handles all three x0x health status spellings ("healthy", "running", "ok") with unit test coverage.

---

Test Coverage Notes

• The Swift reconnect/backoff logic has no unit tests. Error-path behavior (WS drop mid-subscribe, daemon restart, failed resubscription) is not covered.
• GroupsRoundTripTests ban test changed group preset from .privateSecure to .publicOpen — if accidental, this reduces coverage of the privateSecure encryption path. Please add a comment explaining the intent or revert.
• The live-messaging CI job runs CommunitasUITests but the diff does not show those tests being updated to cover the new reconnect paths.

---

Known blocker noted in PR description (x0x v0.22.0 tag not yet released): acknowledged, the CI failure is intentional as a visibility mechanism. No action needed until the x0x tag is cut.

🤖 Generated with Claude Code

[claude]

Code Review — communitas: harden x0x app integration

Good direction overall. The reconnect/backoff, gossip-ACK default, and path-encoding work are all the right calls. A few things worth addressing before merge.

---

Security

Token in WebSocket URL (communitas-x0x-client/src/websocket.rs)

// websocket_url_from_env — line ~16469
Some(format!("{url}?token={}", token.trim()))

The SSE client correctly sends the token as an Authorization: Bearer header via authenticated_client(token), but the WebSocket path embeds the token as a query parameter. Query-string tokens appear in server access logs, browser history, and HTTP Referer headers. Aligning the two transports to the same header-based pattern would remove the inconsistency and the log-exposure risk.

The standard approach for WebSocket auth is to pass the token in the Sec-WebSocket-Protocol subprotocol header or, if the daemon only accepts query params today, to document that constraint explicitly and file a follow-up to fix it on the daemon side.

---

Correctness

No maximum retry bound on WebSocket reconnect (dm_view.rs, channel_chat.rs)

The exponential backoff caps at 30 s, which is fine, but loops indefinitely with no circuit breaker. If the daemon is stopped permanently (e.g. user uninstalls, macOS permission revoked), these tasks spin forever in the background consuming CPU and battery. A simple "after N consecutive failures, mark the stream permanently disconnected and let the user manually reconnect" would be more robust.

retry_delay not reset on re-subscribe failure (channel_chat.rs)

In the channel-chat private-topic path the backoff is correctly reset to 1 s on a successful subscribe, but the public-topic path resets the counter inconsistently — on some error branches retry_delay continues to grow even when the connection succeeds but the subscribe call fails. Worth auditing both branches to make sure the reset is always inside the "we have a working connection" state, not outside it.

---

Style / Conventions

Custom percent-encoder in path_component() (client.rs, X0xClient.swift)

The Rust implementation is correct per RFC 3986, but rolling a bespoke byte-level encoder for a security-relevant function is riskier than using the percent-encoding crate (already in the ecosystem). The project's zero-unwrap standard applies equally to correctness — using a well-audited crate here would be safer. Not a blocker, but worth a follow-up.

Unnamed checkout steps in workflow (.github/workflows/live-messaging.yml)

Both checkout steps in the Dioxus job use uses: actions/checkout@v4 without a name: label. When one fails the log shows two identical step names, which is confusing. Easy fix:

- name: Checkout communitas
  uses: actions/checkout@v4

- name: Checkout x0x
  uses: actions/checkout@v4
  with:
    repository: saorsa-labs/x0x
    ref: ${{ env.X0X_REF }}
    path: x0x

allow-panic-in-tests = true (clippy.toml)

Good addition — consistent with the existing allow-unwrap-in-tests and allow-expect-in-tests policy.

---

Positive changes worth calling out

• Drop impl for X0xWebSocket that aborts background tasks: exactly right.
• peek() before set() guards on Dioxus signals: prevents spurious re-renders.
• live_streams_enabled() guard before entering WebSocket loops: clean way to disable live paths in unit/e2e test mode.
• .absoluteURL fix in Swift X0xWebSocket: resolves a subtle relative-URL resolution bug.
• require_gossip_ack defaulting to true in send_direct: surfacing delivery failures is the right default.
• The known-blocker note about v0.22.0 being undeclared in the PR description and as a workflow comment is good transparency.

---

Summary

The token-in-URL pattern is the only issue that warrants a pre-merge decision (security policy call). The reconnect bound and backoff-reset are correctness nits that should be addressed but could land in a quick follow-up. Everything else is clean.

[claude]

Code Review: PR #26 — communitas: harden x0x app integration

Scope: 124 changed files, +6416 / -6186. Findings ordered by severity.

---

🚫 BLOCKER

1. x0x v0.22.0 tag does not exist — CI tests against v0.21.4 which lacks require_gossip_ack

.github/workflows/live-messaging.yml defaults to v0.21.4. The require_gossip_ack: true field (the primary API change in this PR) will be silently ignored by a v0.21.4 daemon. The live-messaging E2E job will pass while testing a daemon that does not exercise the new behavior — false confidence. Block merge until the x0x v0.22.0 tag exists and the workflow default is updated.

---

🐛 BUGS

2. _enable_polls parameter is received but never used in polling_timers_enabled_for_env()

communitas-dioxus/src/main.rs: The function accepts an enable_polls: bool parameter but its body checks std::env::var("COMMUNITAS_DIOXUS_ENABLE_POLLS") regardless. Passing true or false has no effect; only the env var matters. Either remove the parameter and update call sites, or gate the env-var check with enable_polls && ....

3. Thread panel reconnect loop has no unmount cancellation guard

communitas-dioxus/src/components/thread_panel.rs: The reconnect loop mirrors channel_chat.rs but does not reset a has_connected/has_subscribed guard on unmount (unlike dm_view.rs which resets hasConnected in onDisappear). Closing and re-opening the panel quickly may result in duplicate subscriptions on the same topic.

4. waitForFrame() in Swift tests measures per-frame deadline, not cumulative total

communitas-apple/Tests/X0xClientTests/PubSubRoundTripTests.swift: Each receiveFrame() call resets the timeout, so a loop of N retries can wait up to N × timeoutSeconds. Rename the parameter to perFrameTimeoutSeconds or accumulate elapsed time if a hard total deadline is intended.

5. Optimistic post append in FeedView/SwarmView has no rollback on failure

The change to sync-returning-Bool means the UI appends the post before the async publish settles. If publish() fails, the item stays in the list with no "failed" visual state and was never delivered. For a trust-oriented PQC platform, silent message loss is a risk — at minimum mark failed posts visually and log a warning.

---

🔒 SECURITY

6. Bearer token appended to WebSocket URL as a query parameter

communitas-x0x-client/src/websocket.rs: format!("{url}?token={}", token.trim()) — tokens in query strings appear in server access logs, Referer headers, and browser history. The existing authenticated X0xWebSocket::connect_with_config path correctly uses an Authorization: Bearer header. The env-override path should use the same approach, not a query param. This is a regression from the existing auth pattern.

7. Missing ::add-mask:: for API token in live-messaging.yml

.github/workflows/live-messaging.yml: GitHub masks secrets in logs via ${{ secrets.* }} injection, but if any debug step accidentally echoes the environment, the token appears unmasked. Add an explicit mask step at the start of each job:

- name: Mask API token
  run: echo "::add-mask::${{ secrets.X0X_API_TOKEN }}"

8. home_dir().unwrap_or_default() produces relative paths in containers

communitas-x0x-client/src/daemon.rs: If home dir is unavailable, unwrap_or_default() returns an empty PathBuf, making installed_binary_candidates return paths like .local/bin/x0x (relative, resolved against process CWD). This function is now pub — return an error or skip those candidates when home is unavailable.

---

✅ CORRECTNESS

9. ws_connected initial value is inverted when live streams are disabled

communitas-dioxus/src/components/channel_chat.rs: ws_connected is initialized as !crate::live_streams_enabled() — when live streams are off, the signal starts as true. This is intentional (enables the composer in HTTP-only mode) but the name ws_connected is now misleading. Consider renaming to can_send or composer_enabled.

10. nonce_b64 made Optional without auditing all downstream call sites

communitas-x0x-client/src/types.rs and communitas-apple/Sources/X0xClient/Models/Group.swift: Both make nonce_b64 optional. Any call site that previously accessed .nonce_b64 directly (without guard let / ?) will now silently get nil/None. Audit all usages and propagate Option explicitly or return an error on absence.

11. Asymmetric protocol-translation in sse_url_from_env vs websocket_url_from_env

communitas-x0x-client/src/sse.rs and websocket.rs: sse_url_from_env converts ws:// → http://, while websocket_url_from_env converts http:// → ws://. The logic is architecturally correct but not symmetric. Verify that X0X_API_BASE=ws://127.0.0.1:12700 (typical CI setup) produces correct URLs for both SSE and WS in the live-messaging job.

---

📋 CONVENTION / STYLE

12. set_signal_if_changed() helper duplicated across 5+ components

Identical copies exist in channel_chat.rs, dm_view.rs, swarm_view.rs, feed_view.rs, and thread_panel.rs. Extract to communitas-dioxus/src/utils.rs or a dedicated signals.rs module and import it.

13. Duplicate actions/checkout@v4 step in live-messaging.yml

The Dioxus job has two consecutive checkout steps — the first bare uses: actions/checkout@v4 (no with:) is a copy-paste error. Remove it; only the explicit x0x checkout with ref: should be there.

14. requestPermission() called in both .task and .onAppear

communitas-apple/Sources/Communitas/CommunitasApp.swift: This can trigger two permission dialogs. Remove the .onAppear call and keep only the async .task version.

15. Window startup polling loop runs unconditionally for 10 seconds

communitas-dioxus/src/main.rs: configure_platform_windows_after_startup() polls 100× at 100ms intervals regardless of result. Add a break after a successful configure:

if configure_platform_window().is_ok() { break; }

16. Ban test group preset changed from .privateSecure to .publicOpen without comment

communitas-apple/Tests/X0xClientTests/GroupsRoundTripTests.swift: If the original .privateSecure was testing ban behavior under MLS-encrypted confidentiality, switching to .publicOpen reduces coverage. Add a comment explaining the change.

17. No Swatinem/rust-cache on the Swift live-messaging CI job

.github/workflows/live-messaging.yml: The Dioxus job caches Rust artifacts; the macOS Swift job does not. At minimum cache ~/.build to avoid full recompilation on every scheduled run.

---

⚡ PERFORMANCE

18. AV1 encoder (rav1e) pulled in transitively via dioxus-desktop → image

Cargo.lock: image 0.25.10 enables the avif-encoder feature, pulling in rav1e 0.8.1 + av-scenechange. The desktop app does not encode AVIF. This significantly slows fresh builds. Check if dioxus-desktop's image dependency can have default-features = false in Cargo.toml, and document the libfuzzer-sys blocker in CLAUDE.md under Windows Build Issues.

---

✨ POSITIVE OBSERVATIONS

• X0xWebSocket::Drop impl: Aborting background JoinHandles on drop is the correct fix for the CPU spin. Clean and minimal.
• path_component() URL encoding: RFC 3986 §2.3 compliant custom encoder with three new unit tests — well done.
• Health probe timeout in daemon.rs: Wrapping client.health() in tokio::time::timeout(5s) prevents the onboarding gate from hanging indefinitely.
• from_env_override() pattern: Consistent env-var override across client/SSE/WS for hermetic testing is the right design.
• kanban clippy gate: #![cfg_attr(not(test), deny(...))] + #![cfg_attr(test, allow(...))] is cleaner than per-file #![allow(...)] annotations.
• AppKitFormControls.swift NSViewRepresentable wrappers: Replacing SwiftUI controls that caused macOS Auto Layout crashes is the right fix.

---

Summary

#	Severity	Area	Issue
1	🚫 Blocker	CI	x0x v0.22.0 tag missing; gossip-ack feature untestable
2	🐛 Bug	main.rs	_enable_polls param ignored
3	🐛 Bug	thread_panel.rs	Reconnect loop lacks unmount cancellation
4	🐛 Bug	Swift tests	waitForFrame() per-frame vs total timeout
5	🐛 Bug	feed_view.rs / swarm_view.rs	Optimistic append, no rollback on failure
6	🔒 Security	websocket.rs	Token in WS query param (should be Auth header)
7	🔒 Security	live-messaging.yml	Missing explicit token mask
8	🔒 Security	daemon.rs	home_dir().unwrap_or_default() → relative paths
9	✅ Correctness	channel_chat.rs	ws_connected name misleads in no-live mode
10	✅ Correctness	types.rs / Group.swift	nonce_b64 Optional — call sites unaudited
11	✅ Correctness	sse.rs / websocket.rs	Asymmetric URL protocol translation
12	📋 Convention	5× components	set_signal_if_changed() duplicated
13	📋 Convention	live-messaging.yml	Duplicate checkout step
14	📋 Convention	CommunitasApp.swift	requestPermission() called twice
15	📋 Convention	main.rs	Window polling runs 10s unconditionally
16	📋 Convention	Swift tests	Ban test preset changed without explanation
17	📋 Convention	live-messaging.yml	No cache on Swift CI job
18	⚡ Performance	Cargo.lock	AV1/rav1e pulled in unnecessarily

The security issue (#6, token in query param) and the blocker (#1, missing x0x tag) are the two items that most need resolution before merge.

Reviewed by Claude (claude-sonnet-4-6)