fix(permissions): propagate I/O errors as 500 from delete_agent_permissions

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.
This commit is contained in:
iris 2026-06-27 13:40:34 +02:00 committed by mara
commit 31433da3aa
2 changed files with 24 additions and 3 deletions

View file

@ -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 });

View file

@ -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()
}