From 9bac71b65bf0b26810c215dcdbcd9a022fa5729b Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 6 Aug 2026 19:00:21 -0400 Subject: [PATCH] fix(workspace): represent applied-but-blocked as a truthful third state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provider-access reconciliation runs in the post-commit section of apply_workspace, after relay/keys/scope have committed. The previous .await? propagated any reconciliation failure as a command Err, causing the frontend to treat the workspace as unapplied and clear appliedKey — while the new scope was already active. This contradicted the applied/degraded contract and the useCommunityInit error-handling assumption. Fix: replace .await? with a match that returns WorkspaceApplyResult with applied: true and blocked: Some(reason) on reconciliation failure. Dependent post-commit steps (event sync, agent restore) remain unreached on this path, preserving #4053's fail-closed intent. The frontend parks on the loading gate with a truthful error and the same retry-by-reapply semantics as the catch block. Changes: - scope.rs: add blocked: Option field to WorkspaceApplyResult, add applied_but_blocked() constructor, document the three states - workspace.rs: replace .await? with match; return applied_but_blocked on reconciliation failure; update command doc comment - useCommunityInit.ts: handle blocked after the !applied branch; update catch-block comment (reconcile failures no longer arrive as Err) - tauri.ts: add blocked?: string | null to ApplyWorkspaceResult type - e2eBridge.ts: add blocked: null to apply_workspace mock result - runtime_commands_tests.rs: add three new tests covering the new state and the three-state distinction Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/commands/workspace.rs | 26 ++++++++- .../managed_agents/runtime_commands_tests.rs | 58 +++++++++++++++++++ desktop/src-tauri/src/managed_agents/scope.rs | 35 +++++++++++ .../features/communities/useCommunityInit.ts | 28 ++++++++- desktop/src/shared/api/tauri.ts | 6 +- desktop/src/testing/e2eBridge.ts | 11 +++- 6 files changed, 155 insertions(+), 9 deletions(-) diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index d109a6beb..ef3fe1984 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -92,8 +92,13 @@ pub async fn validate_repos_dir(dir: String) -> Result<(), String> { /// directory. /// /// Returns `WorkspaceApplyResult`: -/// - `applied: true` → new scope committed; post-commit failures surface as -/// `degraded` entries (informational — workspace IS active). +/// - `applied: true`, `blocked: None` → new scope committed; post-commit +/// failures surface as `degraded` entries (informational — workspace IS +/// active). +/// - `applied: true`, `blocked: Some(reason)` → scope committed, but +/// post-commit provider-access reconciliation failed hard; dependent +/// post-commit steps were skipped (fail-closed). Workspace IS active but +/// caller must park on the loading gate — retry by re-applying. /// - `applied: false` → drain failed; old scope still active; `degraded` /// names what could not be stopped or restored by compensation. /// @@ -403,7 +408,22 @@ async fn apply_workspace_body( } let state = restore_app.state::(); - super::agents::provider_access::reconcile_on_workspace_apply(&restore_app, &state).await?; + match super::agents::provider_access::reconcile_on_workspace_apply(&restore_app, &state).await { + Ok(()) => {} + Err(reason) => { + // Provider-access reconciliation failed after the scope committed. + // Preserves #4053's fail-closed intent: no agents spawn against a + // workspace whose provider deployment may not have accepted + // owner-only access. Return applied-but-blocked so the frontend + // can park on the loading gate with a truthful error rather than + // falsely treating the workspace as unapplied. + return Ok( + crate::managed_agents::scope::WorkspaceApplyResult::applied_but_blocked( + reason, degraded, + ), + ); + } + } match crate::managed_agents::retention::active_retention_scope(&restore_app, &state) { Ok(scope) => { diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands_tests.rs b/desktop/src-tauri/src/managed_agents/runtime_commands_tests.rs index 3dd34ff86..9aff81854 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands_tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands_tests.rs @@ -373,6 +373,64 @@ fn test_workspace_apply_result_degradation_accumulates() { assert!(r.degraded[1].contains("sync")); } +#[test] +fn test_workspace_apply_result_applied_but_blocked_sets_fields() { + let r = super::super::scope::WorkspaceApplyResult::applied_but_blocked( + "provider deploy rejected", + Vec::new(), + ); + assert!(r.applied, "applied_but_blocked must report applied: true"); + assert_eq!( + r.blocked.as_deref(), + Some("provider deploy rejected"), + "blocked must carry the failure reason" + ); + assert!(r.degraded.is_empty(), "degraded must be empty when none passed"); +} + +#[test] +fn test_workspace_apply_result_applied_but_blocked_preserves_degraded_entries() { + let prior_degraded = vec!["nest context regeneration failed: io error".to_string()]; + let r = super::super::scope::WorkspaceApplyResult::applied_but_blocked( + "store locked", + prior_degraded, + ); + assert!(r.applied, "applied_but_blocked must report applied: true"); + assert!(r.blocked.is_some(), "blocked must be set"); + assert_eq!(r.degraded.len(), 1, "pre-failure degraded entries must be preserved"); + assert!(r.degraded[0].contains("nest")); +} + +#[test] +fn test_workspace_apply_result_three_state_distinction() { + // State 1: clean success — applied true, blocked None, degraded empty. + let clean = super::super::scope::WorkspaceApplyResult::success(); + assert!(clean.applied); + assert!(clean.blocked.is_none()); + assert!(clean.degraded.is_empty()); + + // State 2: applied with degradation — applied true, blocked None, degraded non-empty. + let degraded = super::super::scope::WorkspaceApplyResult::success() + .with_degradation("event sync skipped"); + assert!(degraded.applied); + assert!(degraded.blocked.is_none()); + assert!(!degraded.degraded.is_empty()); + + // State 3: applied but blocked — applied true, blocked Some, any degraded. + let blocked = super::super::scope::WorkspaceApplyResult::applied_but_blocked( + "provider rejected", + Vec::new(), + ); + assert!(blocked.applied); + assert!(blocked.blocked.is_some()); + + // State 4: drain failed — applied false. + let drained = super::super::scope::WorkspaceApplyResult::drain_failed("lock poisoned"); + assert!(!drained.applied); + assert!(drained.blocked.is_none()); + assert!(!drained.degraded.is_empty()); +} + /// Partial drain: verifies that `execute_drain_journal` delivers the correct /// stopped prefix for compensation. /// diff --git a/desktop/src-tauri/src/managed_agents/scope.rs b/desktop/src-tauri/src/managed_agents/scope.rs index 54a0afb2a..50f019e7a 100644 --- a/desktop/src-tauri/src/managed_agents/scope.rs +++ b/desktop/src-tauri/src/managed_agents/scope.rs @@ -203,6 +203,15 @@ impl WorkspaceAgentScope { } /// Result returned by `apply_workspace` / `import_identity` drain-then-commit. +/// +/// Three distinct states: +/// +/// | `applied` | `blocked` | `degraded` | Meaning | +/// |-----------|--------------|------------|---------| +/// | `true` | `None` | empty | Clean success. | +/// | `true` | `None` | non-empty | Applied with post-commit degradation (workspace IS active; warnings only). | +/// | `true` | `Some(msg)` | any | Applied but blocked: scope committed, post-commit provider reconciliation failed. Workspace IS active; caller must surface this as a hard gate, not a warning, because dependent post-commit steps were skipped to preserve fail-closed behavior. | +/// | `false` | `None` | non-empty | Drain failed; old scope still active. | #[derive(Debug, Clone, serde::Serialize)] pub struct WorkspaceApplyResult { /// `true` when the new scope was committed; `false` when drain or @@ -213,6 +222,16 @@ pub struct WorkspaceApplyResult { /// the degradation is informational. Also populated on drain-failure with /// the specific runtime(s) that could not be stopped or restored. pub degraded: Vec, + /// Set when `applied` is `true` but a post-commit provider-access + /// reconciliation step failed hard. The workspace scope IS committed (the + /// new relay/keys are active), but dependent post-commit steps (event sync, + /// agent restore) were skipped to preserve fail-closed behavior. The caller + /// must treat this as a gate failure — park on the loading screen and + /// surface the reason — because rendering community UI against a workspace + /// whose provider deployment may not have accepted owner-only access is + /// unsafe. Retry by re-applying the workspace. + #[serde(skip_serializing_if = "Option::is_none")] + pub blocked: Option, } impl WorkspaceApplyResult { @@ -220,6 +239,7 @@ impl WorkspaceApplyResult { Self { applied: true, degraded: Vec::new(), + blocked: None, } } @@ -232,6 +252,21 @@ impl WorkspaceApplyResult { Self { applied: false, degraded: vec![msg.into()], + blocked: None, + } + } + + /// Scope committed, but post-commit provider reconciliation failed. + /// + /// `applied` is `true` (the new workspace IS active); `blocked` carries + /// the failure reason. Any `degraded` entries collected before the failure + /// (e.g. nest-regen) are preserved. Dependent post-commit steps must be + /// skipped by the caller after returning this result. + pub fn applied_but_blocked(reason: impl Into, degraded: Vec) -> Self { + Self { + applied: true, + degraded, + blocked: Some(reason.into()), } } } diff --git a/desktop/src/features/communities/useCommunityInit.ts b/desktop/src/features/communities/useCommunityInit.ts index 6519c6754..c5b24b9ff 100644 --- a/desktop/src/features/communities/useCommunityInit.ts +++ b/desktop/src/features/communities/useCommunityInit.ts @@ -254,6 +254,28 @@ export function useCommunityInit( return; } + if (applyResult.blocked != null) { + // Scope committed (applied: true) but post-commit provider-access + // reconciliation failed. The new workspace IS active, but dependent + // steps (event sync, agent restore) were skipped to preserve + // fail-closed behavior. Park on the loading gate so the user can + // retry by re-applying the workspace — same semantics as the catch + // block below, but truthfully reflecting that the workspace committed. + console.error( + "[useCommunityInit] workspace applied but blocked by provider reconciliation failure:", + applyResult.blocked, + ); + if (!cancelled) { + setResult({ + isReady: false, + needsSetup: false, + appliedKey: null, + error: `Workspace applied but provider access configuration failed: ${applyResult.blocked}`, + }); + } + return; + } + // Workspace applied. Surface any post-commit degradation as a // user-visible warning toast — the workspace IS active, but some // best-effort post-commit steps failed (nest, event-sync, restore). @@ -273,8 +295,10 @@ export function useCommunityInit( // it as non-fatal (relay/keys apply, bad value not persisted, REPOS // falls back to a real dir, a `repos-dir-error` toast surfaces it) and // returns Ok, so the app boots into a working state where the user can - // fix the value in community settings. This catch now only fires on a - // genuine relay/key apply failure (e.g. an invalid nsec or a poisoned + // fix the value in community settings. Provider-access reconciliation + // failures also no longer reach here — they arrive as `applied: true` + + // `blocked: string` and are handled above. This catch now only fires on + // a genuine relay/key apply failure (e.g. an invalid nsec or a poisoned // lock). For those, marking the community ready would render // community-scoped UI against a backend that never applied — park on // the loading gate (isReady:false, no appliedKey) instead. diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 792452455..108e646d5 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -1138,7 +1138,11 @@ export async function cancelPairing(): Promise { await invokeTauri("cancel_pairing"); } -type ApplyWorkspaceResult = { applied: boolean; degraded: string[] }; +type ApplyWorkspaceResult = { + applied: boolean; + degraded: string[]; + blocked?: string | null; +}; export async function applyCommunity( relayUrl: string, nsec?: string, diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 98fe6f79e..3324bc499 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -11135,10 +11135,15 @@ export function maybeInstallE2eTauriMocks() { case "fetch_join_policy": return activeConfig?.mock?.joinPolicy ?? null; case "apply_workspace": { - // Must return { applied: boolean, degraded: string[] } — useCommunityInit - // dereferences .applied and .degraded immediately after the await. + // Must return { applied: boolean, degraded: string[], blocked?: string | null } + // — useCommunityInit dereferences .applied, .degraded, and .blocked immediately + // after the await. const applyDelayMs = activeConfig?.mock?.applyCommunityDelayMs ?? 0; - const applyResult = { applied: true, degraded: [] as string[] }; + const applyResult = { + applied: true, + degraded: [] as string[], + blocked: null, + }; if (applyDelayMs > 0) { return new Promise((resolve) => window.setTimeout(() => resolve(applyResult), applyDelayMs),