From 31433da3aa407fa1ca0d728029af331bbb28e722 Mon Sep 17 00:00:00 2001 From: iris Date: Sat, 27 Jun 2026 13:40:34 +0200 Subject: [PATCH] fix(permissions): propagate I/O errors as 500 from delete_agent_permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both remove_agent() calls now run unconditionally for maximum partial cleanup, but any I/O error is returned as HTTP 500 instead of silently 200-ing — so the frontend's !resp.ok path fires and the operator sees a meaningful error rather than the stale row reappearing unchanged. Also add a clarifying comment on isStale in permissions.js explaining that containersState is keyed from nixos-container list (which includes stopped-but-configured containers), so a temporarily-stopped agent is not treated as stale — only destroyed/renamed agents are absent. --- .../packages/dashboard/src/permissions.js | 6 ++++++ hive-c0re/src/dashboard/permissions.rs | 21 ++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/frontend/packages/dashboard/src/permissions.js b/frontend/packages/dashboard/src/permissions.js index 8f1adc25..f1d7bef8 100644 --- a/frontend/packages/dashboard/src/permissions.js +++ b/frontend/packages/dashboard/src/permissions.js @@ -122,6 +122,9 @@ function renderCapabilities(root, data) { // Effective caps (explicit-or-default) drive the checkboxes so // default-perms agents show their real grants, not blank. const assigned = effective[name] || assignments[name] || []; + // `containersState` is keyed from `nixos-container list`, which + // includes stopped-but-configured containers — so a temporarily-stopped + // agent is NOT stale. Only destroyed/renamed agents are absent here. const isStale = !containersState.has(name); const tr = el('tr', { class: 'cap-row' + (isStale ? ' perm-row-stale' : ''), 'data-agent': name }); @@ -226,6 +229,9 @@ function renderToolGroups(root, data) { // badge still keys off explicit-assignment presence. const assigned = effective[name] || assignments[name] || []; const hasExplicit = Object.prototype.hasOwnProperty.call(assignments, name); + // `containersState` is keyed from `nixos-container list`, which + // includes stopped-but-configured containers — only destroyed/renamed + // agents are absent. const isStale = !containersState.has(name); const tr = el('tr', { class: 'tg-row' + (isStale ? ' perm-row-stale' : ''), 'data-agent': name }); diff --git a/hive-c0re/src/dashboard/permissions.rs b/hive-c0re/src/dashboard/permissions.rs index b4d1a8c2..87031e5d 100644 --- a/hive-c0re/src/dashboard/permissions.rs +++ b/hive-c0re/src/dashboard/permissions.rs @@ -332,15 +332,30 @@ pub(super) async fn delete_agent_permissions( if let Some(reason) = validate_agent_name(&logical) { return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response(); } - if let Err(e) = crate::tool_groups::remove_agent(&logical) { + // Run both removals regardless so we clean up as much as possible + // even on partial I/O errors. Collect errors to surface below. + let tg_err = crate::tool_groups::remove_agent(&logical).err(); + if let Some(ref e) = tg_err { tracing::warn!(agent = %logical, error = ?e, "failed to remove tool-groups entry"); } - if let Err(e) = crate::capabilities::remove_agent(&logical) { + let cap_err = crate::capabilities::remove_agent(&logical).err(); + if let Some(ref e) = cap_err { tracing::warn!(agent = %logical, error = ?e, "failed to remove capabilities entry"); } - // Emit live snapshots so the P3RM1SS10NS tab updates immediately. + // Emit live snapshots even on partial failure so the UI stays as + // accurate as possible — the surviving table gets updated immediately. state.coord.emit_tool_groups_snapshot(); state.coord.emit_capabilities_snapshot(); + // Surface any I/O error as 500 so the frontend's `!resp.ok` path + // fires and the operator sees a meaningful message rather than a + // silent "success" followed by the row reappearing unchanged. + if let Some(e) = tg_err.or(cap_err) { + return ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("failed to remove permission entries for {logical}: {e}"), + ) + .into_response(); + } tracing::info!(agent = %logical, "operator: cleared stale permission entries via dashboard"); (StatusCode::OK, "ok").into_response() }