mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(workspace): represent applied-but-blocked as a truthful third state
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<String> 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 <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -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::<AppState>();
|
||||
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) => {
|
||||
|
||||
@@ -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.
|
||||
///
|
||||
|
||||
@@ -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<String>,
|
||||
/// 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<String>,
|
||||
}
|
||||
|
||||
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<String>, degraded: Vec<String>) -> Self {
|
||||
Self {
|
||||
applied: true,
|
||||
degraded,
|
||||
blocked: Some(reason.into()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -1138,7 +1138,11 @@ export async function cancelPairing(): Promise<void> {
|
||||
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,
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user