Skip to content

Fix false "relay unreachable" error on agent creation #1433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
matbalez wants to merge 2 commits into
base: main
Choose a base branch
from
+40 −2
Open

Fix false "relay unreachable" error on agent creation #1433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4438877
Fix false "relay unreachable" error on agent creation
matbalez Jul 1, 2026
74651f5
Address review: trim verbose comments, fix guard-comment accuracy
matbalez Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion desktop/src-tauri/src/commands/agents.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,9 +886,14 @@ pub async fn create_managed_agent(
// ── Phase 4: sync agent profile on relay (async, outside lock) ───────────
// Use the avatar persisted on the record so the published profile and any
// later reconciliation agree on the same value.
// Blank per-agent relay falls back to the workspace relay (matches reconcile/rename).
let sync_relay_url = crate::relay::effective_agent_relay_url(
&resolved_relay_url,
&relay_ws_url_with_override(&state),
);
let profile_sync_error = (sync_managed_agent_profile(
&state,
&resolved_relay_url,
&sync_relay_url,
&agent_keys,
&name,
resolved_avatar_url.as_deref(),
Expand Down
35 changes: 34 additions & 1 deletion desktop/src-tauri/src/relay.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,12 +382,21 @@ pub async fn sync_managed_agent_profile(
avatar_url: Option<&str>,
auth_tag: Option<&str>, // NIP-OA auth tag JSON
) -> Result<(), String> {
// Fail loudly on a blank relay URL — callers must resolve via `effective_agent_relay_url` first.
let base = relay_http_base_url(relay_url);
if base.is_empty() {
return Err(

@wesbillman wesbillman Jul 1, 2026

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.

Two small things:

  1. The comment says "blank/hostless", but only blank is caught. A hostless-but-schemed input slips through: relay_http_base_url("wss://") returns "wss:" (the trailing-slash trim defeats the prefix strip), which is non-empty and sails past this guard into the same misclassified connect failure. Pre-existing edge and fine to leave unfixed — but the comment shouldn't claim coverage it doesn't have.
  2. This 6-line block restates the PR description; house style is the why lives in the PR, ≤1 short line in code. Something like // Fail loudly on a blank relay URL — callers must resolve via effective_agent_relay_url first. carries the load.

Nit: the error string surfaces verbatim in the user-facing toast. Only reachable if a future caller regresses, so acceptable as a loud dev-facing failure.

@matbalez matbalez Jul 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 74651f5. Trimmed to one line and dropped the inaccurate "blank/hostless" wording — it now reads // Fail loudly on a blank relay URL — callers must resolve via effective_agent_relay_url first. Good catch on the wss:// → wss: edge; left the pre-existing behavior as-is since it is out of scope for this fix.

"no relay configured for profile sync (resolve the agent's relay URL first)"
.to_string(),
);
}

// Build a signed kind:0 profile event (with optional NIP-OA auth tag).
let event = build_profile_event(agent_keys, display_name, avatar_url, auth_tag)?;
let event_json = event.as_json();
let body_bytes = event_json.into_bytes();

let url = format!("{}/events", relay_http_base_url(relay_url));
let url = format!("{}/events", base);
let auth = build_nip98_auth_header_for_keys(agent_keys, &Method::POST, &url, &body_bytes)?;

let mut request = state
Expand Down Expand Up @@ -708,6 +717,30 @@ mod tests {
);
}

#[test]
fn blank_relay_http_base_is_empty() {
// A blank relay URL has no scheme to rewrite, so the HTTP base is empty.
assert_eq!(relay_http_base_url(""), "");
assert_eq!(relay_http_base_url(" "), "");
}

#[test]
fn ui_created_agent_blank_relay_resolves_to_reachable_host() {

@wesbillman wesbillman Jul 1, 2026

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.

This test does not fail against the pre-fix logic, contrary to the PR body. It only composes effective_agent_relay_url + relay_http_base_url — two pure helpers unchanged by this PR and already covered individually by empty_override_uses_workspace_relay and the base-URL suite. The pre-fix bug lived in the caller (create_managed_agent Phase 4 not invoking the helper), which no unit test exercises.

Not blocking — Phase 4 is a Tauri command that's genuinely hard to unit-test, and the new guard in sync_managed_agent_profile is the real regression protection here. But please correct the PR body's "Fails against the pre-fix logic" claim, and consider trimming this test's comment to one line — as written it documents the fix rather than what the test verifies.

@matbalez matbalez Jul 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 74651f5. Both test comments now describe what the test verifies in one line, not the fix. Also corrected the PR body: the resolution test no longer claims it "fails against the pre-fix logic" — it pins the resolution contract (two pure helpers), and the real regression protection is called out as the blank-relay guard in sync_managed_agent_profile.

// A blank per-agent relay (the UI create case) resolves through
// `effective_agent_relay_url` to a real workspace host, not a hostless base.
let resolved_relay_url = ""; // UI create flow: no per-agent relay pinned
let workspace_relay = "wss://staging.example.com";

let sync_relay_url = effective_agent_relay_url(resolved_relay_url, workspace_relay);
let base = relay_http_base_url(&sync_relay_url);

assert!(
!base.is_empty(),
"blank per-agent relay must resolve to a real workspace host, not a hostless base"
);
assert_eq!(base, "https://staging.example.com");
}

// ── classify_intercepted_response ────────────────────────────────────────

#[test]
Expand Down