From e6a68b2ef12cb56f5725ad78da7f866f2670e740 Mon Sep 17 00:00:00 2001 From: scanash00 Date: Wed, 29 Jul 2026 23:46:05 -0800 Subject: [PATCH] oopsie fix + more frontend improvements --- .../src/repo/record/validation.rs | 43 ++++- crates/tranquil-lexicon/src/dynamic.rs | 46 ++++- crates/tranquil-lexicon/src/resolve.rs | 29 +++ .../tests/resolve_integration.rs | 4 + .../dashboard/ControllersContent.svelte | 173 ++++++++++-------- frontend/src/lib/types/api.ts | 1 + frontend/src/styles/dashboard.css | 72 +++----- 7 files changed, 239 insertions(+), 129 deletions(-) diff --git a/crates/tranquil-api/src/repo/record/validation.rs b/crates/tranquil-api/src/repo/record/validation.rs index ecabeb0..b009412 100644 --- a/crates/tranquil-api/src/repo/record/validation.rs +++ b/crates/tranquil-api/src/repo/record/validation.rs @@ -9,16 +9,34 @@ pub async fn validate_record_with_status( require_lexicon: bool, ) -> Result { let registry = tranquil_lexicon::LexiconRegistry::global(); + let mut resolution_error = None; if !registry.has_schema(collection) { - let _ = registry.resolve_dynamic(collection).await; + if let Err(error) = registry.resolve_dynamic(collection).await { + tracing::warn!( + collection = %collection, + error = %error, + "could not resolve record lexicon" + ); + resolution_error = Some(error); + } } - let validator = RecordValidator::new().require_lexicon(require_lexicon); + let validator = RecordValidator::new().require_lexicon(should_require_lexicon( + require_lexicon, + resolution_error.as_ref(), + )); validator .validate_with_rkey(record, collection, rkey) .map_err(validation_error_to_api_error) } +fn should_require_lexicon( + requested: bool, + resolution_error: Option<&tranquil_lexicon::ResolveError>, +) -> bool { + requested && !resolution_error.is_some_and(tranquil_lexicon::ResolveError::is_transient) +} + fn validation_error_to_api_error(e: ValidationError) -> ApiError { let msg = match e { ValidationError::MissingType => "Record must have a $type field".to_string(), @@ -43,3 +61,24 @@ fn validation_error_to_api_error(e: ValidationError) -> ApiError { }; ApiError::InvalidRecord(msg) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strict_validation_fails_open_only_for_transient_resolution_errors() { + let transient = tranquil_lexicon::ResolveError::DnsLookup { + domain: "feed.bsky.app".to_string(), + reason: "temporary failure".to_string(), + }; + let definitive = tranquil_lexicon::ResolveError::NoDid { + domain: "example.com".to_string(), + }; + + assert!(!should_require_lexicon(true, Some(&transient))); + assert!(should_require_lexicon(true, Some(&definitive))); + assert!(should_require_lexicon(true, None)); + assert!(!should_require_lexicon(false, None)); + } +} diff --git a/crates/tranquil-lexicon/src/dynamic.rs b/crates/tranquil-lexicon/src/dynamic.rs index 96581e2..16c1693 100644 --- a/crates/tranquil-lexicon/src/dynamic.rs +++ b/crates/tranquil-lexicon/src/dynamic.rs @@ -244,8 +244,10 @@ impl DynamicRegistry { Some(_guard) => match resolver(nsid.clone()).await { Ok(doc) => Ok(self.insert_schema(doc)), Err(e) => { - self.insert_negative(nsid); - tracing::debug!(nsid = %nsid, error = %e, "caching negative resolution result"); + if !e.is_transient() { + self.insert_negative(nsid); + tracing::debug!(nsid = %nsid, error = %e, "caching negative resolution result"); + } Err(e) } }, @@ -347,6 +349,41 @@ mod tests { ); } + #[tokio::test] + async fn transient_resolution_failure_is_not_negative_cached() { + let registry = DynamicRegistry::new(); + let collection = nsid("app.bsky.feed.post"); + + let result = registry + .resolve_and_cache_with(&collection, |_| async { + Err(ResolveError::DnsLookup { + domain: "feed.bsky.app".to_string(), + reason: "temporary failure".to_string(), + }) + }) + .await; + + assert!(matches!(result, Err(ResolveError::DnsLookup { .. }))); + assert!(!registry.is_negative_cached(&collection)); + } + + #[tokio::test] + async fn definitive_resolution_failure_is_negative_cached() { + let registry = DynamicRegistry::new(); + let collection = nsid("com.example.missing"); + + let result = registry + .resolve_and_cache_with(&collection, |_| async { + Err(ResolveError::NoDid { + domain: "example.com".to_string(), + }) + }) + .await; + + assert!(matches!(result, Err(ResolveError::NoDid { .. }))); + assert!(registry.is_negative_cached(&collection)); + } + #[test] fn test_empty_lookup() { let registry = DynamicRegistry::new(); @@ -616,7 +653,10 @@ mod tests { 1, "single-flight must coalesce failing resolves too" ); - assert!(registry.is_negative_cached(&nsid("pet.nel.failHerd"))); + assert!( + !registry.is_negative_cached(&nsid("pet.nel.failHerd")), + "transient leader failures must remain retryable" + ); } async fn futures_collect(handles: Vec>) -> Vec { diff --git a/crates/tranquil-lexicon/src/resolve.rs b/crates/tranquil-lexicon/src/resolve.rs index 4662223..08377da 100644 --- a/crates/tranquil-lexicon/src/resolve.rs +++ b/crates/tranquil-lexicon/src/resolve.rs @@ -63,6 +63,10 @@ pub enum ResolveError { NoPdsEndpoint { did: Did }, #[error("schema fetch failed from {url}: {reason}")] SchemaFetch { url: String, reason: String }, + #[error("schema not found at {url}")] + SchemaNotFound { url: String }, + #[error("schema request to {url} was rejected with HTTP {status}")] + SchemaRejected { url: String, status: u16 }, #[error("schema deserialization failed: {0}")] InvalidSchema(String), #[error("schema resolution recently failed for {nsid}, cached for {ttl_secs}s")] @@ -73,6 +77,19 @@ pub enum ResolveError { LeaderAborted { nsid: Nsid }, } +impl ResolveError { + pub fn is_transient(&self) -> bool { + matches!( + self, + Self::DnsLookup { .. } + | Self::DidResolution { .. } + | Self::SchemaFetch { .. } + | Self::NetworkDisabled + | Self::LeaderAborted { .. } + ) + } +} + pub fn nsid_to_authority(nsid: &Nsid) -> String { let mut segments: Vec<&str> = nsid.split('.').collect(); segments.pop(); @@ -204,6 +221,18 @@ pub async fn fetch_schema_from_pds( let status = resp.status(); if !status.is_success() { + if status == reqwest::StatusCode::NOT_FOUND { + return Err(ResolveError::SchemaNotFound { url }); + } + if status.is_client_error() + && status != reqwest::StatusCode::REQUEST_TIMEOUT + && status != reqwest::StatusCode::TOO_MANY_REQUESTS + { + return Err(ResolveError::SchemaRejected { + url, + status: status.as_u16(), + }); + } return Err(ResolveError::SchemaFetch { url, reason: format!("HTTP {}", status), diff --git a/crates/tranquil-lexicon/tests/resolve_integration.rs b/crates/tranquil-lexicon/tests/resolve_integration.rs index 68158ee..1a8d224 100644 --- a/crates/tranquil-lexicon/tests/resolve_integration.rs +++ b/crates/tranquil-lexicon/tests/resolve_integration.rs @@ -383,6 +383,10 @@ async fn test_fetch_schema_error_status_gives_meaningful_error() { ) .await; let err = result.unwrap_err(); + assert!(matches!( + &err, + ResolveError::SchemaRejected { status: 400, .. } + )); let err_msg = err.to_string(); assert!( !err_msg.contains("missing 'value' field"), diff --git a/frontend/src/components/dashboard/ControllersContent.svelte b/frontend/src/components/dashboard/ControllersContent.svelte index 1120ae9..1546e67 100644 --- a/frontend/src/components/dashboard/ControllersContent.svelte +++ b/frontend/src/components/dashboard/ControllersContent.svelte @@ -57,47 +57,16 @@ let resolvedController = $state<{ did: string; handle?: string; pdsUrl?: string; isLocal: boolean } | null>(null) let resolving = $state(false) let resolveError = $state('') + let controllerToRemove = $state(null) + let removeDialog = $state() + let removingController = $state(false) - let typeaheadResults = $state>([]) - let typeaheadTimeout: ReturnType | null = null - let showTypeahead = $state(false) + let selectedControllerPreset = $derived(scopePresets.find(p => p.scopes === addControllerScopes)) function onControllerInput(value: string) { addControllerIdentifier = value resolvedController = null resolveError = '' - - if (typeaheadTimeout) clearTimeout(typeaheadTimeout) - - const trimmed = value.trim().replace(/^@/, '') - if (trimmed.startsWith('did:') || trimmed.length < 2) { - typeaheadResults = [] - showTypeahead = false - return - } - - typeaheadTimeout = setTimeout(async () => { - const resp = await fetch( - `https://public.api.bsky.app/xrpc/app.bsky.actor.searchActorsTypeahead?q=${encodeURIComponent(trimmed)}&limit=5` - ) - if (resp.ok) { - const data = await resp.json() - typeaheadResults = (data.actors ?? []).map((a: Record) => ({ - did: a.did as string, - handle: a.handle as string, - displayName: a.displayName as string | undefined, - avatar: a.avatar as string | undefined, - })) - showTypeahead = typeaheadResults.length > 0 - } - }, 200) - } - - function selectTypeahead(actor: { did: string; handle: string }) { - addControllerIdentifier = actor.handle - showTypeahead = false - typeaheadResults = [] - resolveControllerIdentifier() } async function resolveControllerIdentifier() { @@ -122,6 +91,7 @@ let newDelegatedEmail = $state('') let newDelegatedScopes = $state('') let creatingDelegated = $state(false) + let selectedDelegatedPreset = $derived(scopePresets.find(p => p.scopes === newDelegatedScopes)) onMount(async () => { await Promise.all([loadData(), loadAuditLog()]) @@ -165,7 +135,7 @@ if (result.ok) { scopePresets = (result.value.presets ?? []).map((p: DelegationScopePreset) => ({ name: p.name, - label: p.name, + label: p.label, description: p.description, scopes: unsafeAsScopeSet(p.scopes) })) @@ -190,18 +160,46 @@ resolvedController = null showAddController = false await loadControllers() + } else { + toast.error(result.error.message || $_('common.error')) } addingController = false } - async function removeController(controllerDid: Did) { - if (!confirm($_('delegation.removeConfirm'))) return + function requestControllerRemoval(controller: Controller) { + controllerToRemove = controller + } + + function cancelControllerRemoval() { + if (removingController) return + controllerToRemove = null + } + + $effect(() => { + const currentDialog = removeDialog + if (!currentDialog) return + if (controllerToRemove && !currentDialog.open) { + currentDialog.showModal() + requestAnimationFrame(() => currentDialog.querySelector('.cancel-remove-controller')?.focus()) + } else if (!controllerToRemove && currentDialog.open) { + currentDialog.close() + } + }) + + async function removeController() { + if (!controllerToRemove) return + const controllerDid = controllerToRemove.did + removingController = true const result = await api.removeDelegationController(session.accessJwt, controllerDid) if (result.ok) { toast.success($_('delegation.controllerRemoved')) + controllerToRemove = null await loadControllers() + } else { + toast.error(result.error.message || $_('common.error')) } + removingController = false } async function createDelegatedAccount() { @@ -221,6 +219,8 @@ newDelegatedScopes = defaultScopes showCreateDelegated = false await loadControlledAccounts() + } else { + toast.error(result.error.message || $_('common.error')) } creatingDelegated = false } @@ -389,7 +389,7 @@
-
@@ -403,7 +403,7 @@

{$_('delegation.cannotAddControllers')}

{:else if showAddController} -
+
{ event.preventDefault(); addController() }}>

{$_('delegation.addController')}

@@ -418,41 +418,34 @@
-
+
onControllerInput((e.target as HTMLInputElement).value)} - onblur={() => { setTimeout(() => { showTypeahead = false }, 200) }} - onkeydown={(e) => { if (e.key === 'Enter') { e.preventDefault(); showTypeahead = false; resolveControllerIdentifier() } }} placeholder="handle or did:plc:..." disabled={addingController} + required + aria-invalid={resolveError ? 'true' : undefined} + aria-describedby={resolving || resolveError || resolvedController ? 'controllerResolveStatus' : undefined} /> - {#if showTypeahead && typeaheadResults.length > 0} -
- {#each typeaheadResults as actor} - - {/each} -
- {/if} +
{#if resolving} - {$_('common.loading')} + {$_('common.loading')} {:else if resolveError} - {resolveError} + {resolveError} {:else if resolvedController} -
+
{resolvedController.did} {#if resolvedController.handle} @{resolvedController.handle} @@ -465,25 +458,28 @@
- {#each scopePresets as preset} - + {/each} + {#if selectedControllerPreset} + {selectedControllerPreset.description} + {/if}
-
-
+ {:else}
{:else if showCreateDelegated} -
+
{ event.preventDefault(); createDelegatedAccount() }}>

{$_('delegation.createDelegatedAccount')}

- {#each scopePresets as preset} - + {/each} + {#if selectedDelegatedPreset} + {selectedDelegatedPreset.description} + {/if}
-
-
+
{:else}
+ +{#if controllerToRemove} + { event.preventDefault(); cancelControllerRemoval() }} + onclick={(event) => event.target === removeDialog && cancelControllerRemoval()} + > +

{$_('delegation.remove')}

+

{$_('delegation.removeConfirm')}

+

+ {controllerToRemove.handle ? `@${controllerToRemove.handle}` : controllerToRemove.did} +

+
+ + +
+ +{/if} diff --git a/frontend/src/lib/types/api.ts b/frontend/src/lib/types/api.ts index 4f3b492..13a3041 100644 --- a/frontend/src/lib/types/api.ts +++ b/frontend/src/lib/types/api.ts @@ -545,6 +545,7 @@ export interface DelegationControlledAccount { export interface DelegationScopePreset { name: string; + label: string; scopes: ScopeSet; description: string; } diff --git a/frontend/src/styles/dashboard.css b/frontend/src/styles/dashboard.css index edb1fdd..3d19dcd 100644 --- a/frontend/src/styles/dashboard.css +++ b/frontend/src/styles/dashboard.css @@ -1895,65 +1895,19 @@ a.btn-link:hover { position: relative; } -.search-wrapper { - position: relative; -} - -.typeahead-dropdown { - position: absolute; - top: 100%; - left: 0; - right: 0; - z-index: 10; - background: var(--bg-card); - box-shadow: var(--shadow-lg); - max-height: 240px; - overflow-y: auto; -} - -.typeahead-item { +.controller-lookup-row { display: flex; align-items: center; gap: var(--space-2); - width: 100%; - padding: var(--space-2) var(--space-3); - border: none; - background: transparent; - cursor: pointer; - text-align: left; - color: var(--text-primary); } -button.typeahead-item:hover { - background: var(--bg-tertiary); -} - -.typeahead-avatar { - width: 28px; - height: 28px; - flex-shrink: 0; -} - -.typeahead-text { - display: flex; - flex-direction: column; +.controller-lookup-row input { + flex: 1; min-width: 0; } -.typeahead-name { - font-size: var(--text-sm); - font-weight: var(--font-medium); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; -} - -.typeahead-handle { - font-size: var(--text-xs); - color: var(--text-secondary); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; +.controller-lookup-row button { + flex-shrink: 0; } .resolve-status { @@ -2037,7 +1991,23 @@ button.typeahead-item:hover { word-break: break-word; } +.controller-removal-dialog h2 { + margin-top: 0; +} + +.controller-removal-identity { + padding: var(--space-3); + background: var(--bg-tertiary); + font-family: var(--font-mono); + overflow-wrap: anywhere; +} + @media (max-width: 600px) { + .controller-lookup-row { + align-items: stretch; + flex-direction: column; + } + .item-card { flex-direction: column; align-items: stretch; -- 2.51.2