From 60e1f40562dcad27f46e539829e7d568025374b0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 10 Aug 2026 04:47:33 -0700 Subject: [PATCH] refactor: replace exec approvals lease with journal CAS (#121273) * refactor: replace exec approvals lease with journal CAS * style: format Swift exec approvals loop * test: mutate journaled agent in native fence coverage * fix: normalize native exec approval fence IDs * fix: remove exec approval aliases on agent deletion --- .../ExecApprovalsSQLiteStore.swift | 58 ++++-- .../ExecApprovalsSQLiteStoreTests.swift | 52 ++--- src/infra/exec-approvals-sqlite.ts | 100 +++++---- src/infra/exec-approvals-store.test.ts | 64 +++--- src/infra/exec-approvals-store.ts | 192 +++++++----------- 5 files changed, 250 insertions(+), 216 deletions(-) diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/ExecApprovalsSQLiteStore.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/ExecApprovalsSQLiteStore.swift index 8cf3aa499ea1..47a3af751418 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/ExecApprovalsSQLiteStore.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/ExecApprovalsSQLiteStore.swift @@ -193,8 +193,6 @@ public struct ExecApprovalsSQLiteMutation { public enum ExecApprovalsSQLiteStore { public static let configKey = "current" public static let locator = "state/openclaw.sqlite#exec_approvals_config" - static let mutationLeaseScope = "exec-approvals" - static let mutationLeaseKey = "mutation" private static let busyTimeoutMilliseconds: Int32 = 30000 public static func databaseURL(stateDirectoryURL: URL) -> URL { @@ -234,9 +232,13 @@ public enum ExecApprovalsSQLiteStore { let database = try self.openDatabase(stateDirectoryURL: stateDirectoryURL) return try database.withImmediateTransaction { try database.ensureCanonicalTable(.execApprovalsConfig) - let mutation = try body(self.readRecord(database)) + let current = try self.readRecord(database) + let mutation = try body(current) if let document = mutation.documentToWrite { - try self.assertMutationNotFenced(database) + try self.assertMutationNotFenced( + database, + current: current?.document, + next: document) try self.writeRecord( database, document: document, @@ -342,28 +344,46 @@ public enum ExecApprovalsSQLiteStore { } private static func assertMutationNotFenced( - _ database: OpenClawNativeStateSQLite) throws + _ database: OpenClawNativeStateSQLite, + current: ExecApprovalsDocument?, + next: ExecApprovalsDocument) throws { let table = try database.prepare( - "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'state_leases'") + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'agent_deletion_journal'") guard try table.step() == .row else { return } - let statement = try database.prepare(""" - SELECT owner FROM state_leases - WHERE scope = ? AND lease_key = ? AND expires_at > ? - LIMIT 1 - """) - try statement.bindText(self.mutationLeaseScope, at: 1) - try statement.bindText(self.mutationLeaseKey, at: 2) - try statement.bindInt64(Int64(Date().timeIntervalSince1970 * 1000), at: 3) - // Expired rows intentionally do not fence writers: TTL expiry is the - // crash-release path when the deleting process cannot remove its lease. - guard try statement.step() != .row else { - throw OpenClawNativeStateError( - "Exec approvals cannot be changed while agent deletion is in progress; retry.") + let currentAgents = current.map { self.projectionDocument($0).agents ?? [:] } ?? [:] + let nextAgents = self.projectionDocument(next).agents ?? [:] + for agentID in Set(currentAgents.keys).union(nextAgents.keys) + where currentAgents[agentID] != nextAgents[agentID] + { + let statement = try database.prepare( + "SELECT 1 FROM agent_deletion_journal WHERE agent_id = ? LIMIT 1") + try statement.bindText(self.normalizedAgentID(agentID), at: 1) + if try statement.step() == .row { + throw OpenClawNativeStateError( + "Exec approvals cannot be changed while agent deletion is in progress; retry.") + } } } + private static func normalizedAgentID(_ value: String) -> String { + let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return "main" } + let normalized = trimmed.lowercased() + if trimmed.range( + of: "^[a-z0-9][a-z0-9_-]{0,63}$", + options: [.regularExpression, .caseInsensitive]) != nil + { + return normalized + } + let replaced = normalized.replacingOccurrences( + of: "[^a-z0-9_-]+", with: "-", options: .regularExpression) + let stripped = replaced.trimmingCharacters(in: CharacterSet(charactersIn: "-")) + let truncated = String(stripped.prefix(64)) + return truncated.isEmpty ? "main" : truncated + } + private static func projectionDocument( _ document: ExecApprovalsDocument) -> ExecApprovalsDocument { diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ExecApprovalsSQLiteStoreTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ExecApprovalsSQLiteStoreTests.swift index bd7580a27dc0..795ace21dabb 100644 --- a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ExecApprovalsSQLiteStoreTests.swift +++ b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ExecApprovalsSQLiteStoreTests.swift @@ -73,13 +73,14 @@ struct ExecApprovalsSQLiteStoreTests { } @Test - func `active deletion lease fences writes until expiry`() throws { + func `deletion journal fences only the affected agent`() throws { try self.withStateDirectory { stateDirectoryURL in - let original = Self.document(token: "original", agentCount: 1) + var original = Self.document(token: "original", agentCount: 1) + let originalAgent = original.agents?.removeValue(forKey: "agent-0") + original.agents?["Agent A"] = originalAgent try ExecApprovalsSQLiteStore.write(original, stateDirectoryURL: stateDirectoryURL) let databaseURL = ExecApprovalsSQLiteStore.databaseURL( stateDirectoryURL: stateDirectoryURL) - let expiresAt = Int64(Date().timeIntervalSince1970 * 1000) + 120_000 try Self.execute(databaseURL, """ CREATE TABLE schema_meta ( meta_key TEXT NOT NULL PRIMARY KEY, @@ -94,41 +95,46 @@ struct ExecApprovalsSQLiteStoreTests { meta_key, role, schema_version, agent_id, app_version, created_at, updated_at ) VALUES ('primary', 'global', 6, NULL, NULL, 1, 1); - CREATE TABLE state_leases ( - scope TEXT NOT NULL, - lease_key TEXT NOT NULL, - owner TEXT NOT NULL, - expires_at INTEGER, - heartbeat_at INTEGER, - payload_json TEXT, + CREATE TABLE agent_deletion_journal ( + agent_id TEXT PRIMARY KEY, + operation_id TEXT NOT NULL DEFAULT '', + agent_dir TEXT NOT NULL, + workspace_dir TEXT NOT NULL, + sessions_dir TEXT NOT NULL, + database_paths_json TEXT NOT NULL DEFAULT '[]', + cleanup_paths_json TEXT NOT NULL DEFAULT '[]', created_at INTEGER NOT NULL, - updated_at INTEGER NOT NULL, - PRIMARY KEY (scope, lease_key) + cleanup_completed INTEGER NOT NULL DEFAULT 0, + delete_files INTEGER NOT NULL DEFAULT 1 ) STRICT; - INSERT INTO state_leases ( - scope, lease_key, owner, expires_at, heartbeat_at, - payload_json, created_at, updated_at - ) VALUES ( - '\(ExecApprovalsSQLiteStore.mutationLeaseScope)', - '\(ExecApprovalsSQLiteStore.mutationLeaseKey)', - 'typescript-deletion', \(expiresAt), 1, NULL, 1, 1 - ); + INSERT INTO agent_deletion_journal ( + agent_id, operation_id, agent_dir, workspace_dir, sessions_dir, created_at + ) VALUES ('agent-a', 'typescript-deletion', '/agent', '/workspace', '/sessions', 1); PRAGMA user_version = 6; """) - let replacement = Self.document(token: "replacement", agentCount: 2) + var replacement = Self.document(token: "replacement", agentCount: 2) + let replacementAgent = replacement.agents?.removeValue(forKey: "agent-0") + replacement.agents?["Agent A"] = replacementAgent + replacement.agents?["Agent A"]?.security = .deny do { try ExecApprovalsSQLiteStore.write( replacement, stateDirectoryURL: stateDirectoryURL) - Issue.record("Expected active agent deletion lease to fence the write") + Issue.record("Expected active agent deletion journal to fence the write") } catch { #expect(error.localizedDescription.contains("agent deletion is in progress; retry")) } #expect(try ExecApprovalsSQLiteStore.read( stateDirectoryURL: stateDirectoryURL)?.document == original) - try Self.execute(databaseURL, "UPDATE state_leases SET expires_at = 0") + var unrelated = original + unrelated.socket?.token = "unrelated" + try ExecApprovalsSQLiteStore.write(unrelated, stateDirectoryURL: stateDirectoryURL) + #expect(try ExecApprovalsSQLiteStore.read( + stateDirectoryURL: stateDirectoryURL)?.document == unrelated) + + try Self.execute(databaseURL, "DELETE FROM agent_deletion_journal") try ExecApprovalsSQLiteStore.write( replacement, stateDirectoryURL: stateDirectoryURL) diff --git a/src/infra/exec-approvals-sqlite.ts b/src/infra/exec-approvals-sqlite.ts index 468e6901ecc1..69953497b2f2 100644 --- a/src/infra/exec-approvals-sqlite.ts +++ b/src/infra/exec-approvals-sqlite.ts @@ -1,7 +1,8 @@ // Canonical SQLite row helpers for exec approval policy state. import type { DatabaseSync } from "node:sqlite"; +import { isDeepStrictEqual } from "node:util"; +import { normalizeAgentId } from "../routing/session-key.js"; import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js"; -import type { OpenClawStateLeaseContext } from "../state/openclaw-state-lease.js"; import { sha256Hex } from "./crypto-digest.js"; import { normalizeExecApprovalsInternal, @@ -15,45 +16,82 @@ import { } from "./kysely-sync.js"; const EXEC_APPROVALS_CONFIG_KEY = "current"; -export const EXEC_APPROVALS_MUTATION_LEASE_SCOPE = "exec-approvals"; -export const EXEC_APPROVALS_MUTATION_LEASE_KEY = "mutation"; type ExecApprovalsDatabase = Pick< OpenClawStateKyselyDatabase, - "exec_approvals_config" | "state_leases" + "agent_deletion_journal" | "exec_approvals_config" >; -export type ExecApprovalsMutationLeaseOwner = Pick< - OpenClawStateLeaseContext, - "assertOwnedInTransaction" ->; +export type ExecApprovalsMutationAuthority = { + action: "remove" | "restore"; + agentId: string; + operationId: string; +}; -class ExecApprovalsMutationFencedError extends Error { +export class ExecApprovalsMutationFencedError extends Error { constructor() { super("Exec approvals cannot be changed while agent deletion is in progress; retry."); this.name = "ExecApprovalsMutationFencedError"; } } -function assertExecApprovalsMutationAllowed(params: { - db: DatabaseSync; - leaseOwner?: ExecApprovalsMutationLeaseOwner; - now?: number; -}): void { - if (params.leaseOwner) { - params.leaseOwner.assertOwnedInTransaction(params.db); - return; - } - const activeLease = executeSqliteQueryTakeFirstSync( - params.db, - getNodeSqliteKysely(params.db) - .selectFrom("state_leases") - .select("owner") - .where("scope", "=", EXEC_APPROVALS_MUTATION_LEASE_SCOPE) - .where("lease_key", "=", EXEC_APPROVALS_MUTATION_LEASE_KEY) - .where("expires_at", ">", params.now ?? Date.now()), +export function assertExecApprovalsMutationAuthority( + db: DatabaseSync, + authority: ExecApprovalsMutationAuthority, +): void { + const journal = executeSqliteQueryTakeFirstSync( + db, + getNodeSqliteKysely(db) + .selectFrom("agent_deletion_journal") + .select("operation_id") + .where("agent_id", "=", normalizeAgentId(authority.agentId)), ); - if (activeLease) { + if (journal?.operation_id !== authority.operationId) { + throw new ExecApprovalsMutationFencedError(); + } +} + +export function assertExecApprovalsMutationAllowed(params: { + db: DatabaseSync; + current: ExecApprovalsFile; + next: ExecApprovalsFile; + authority?: ExecApprovalsMutationAuthority; +}): void { + const current = normalizeExecApprovalsInternal(params.current); + const next = normalizeExecApprovalsInternal(params.next); + const agentIds = new Set([ + ...Object.keys(current.agents ?? {}), + ...Object.keys(next.agents ?? {}), + ]); + const state = getNodeSqliteKysely(params.db); + for (const agentId of agentIds) { + const currentPolicy = current.agents?.[agentId]; + const nextPolicy = next.agents?.[agentId]; + if (isDeepStrictEqual(currentPolicy, nextPolicy)) { + continue; + } + const normalizedAgentId = normalizeAgentId(agentId); + const journal = executeSqliteQueryTakeFirstSync( + params.db, + state + .selectFrom("agent_deletion_journal") + .select("operation_id") + .where("agent_id", "=", normalizedAgentId), + ); + if (!journal) { + continue; + } + const authority = params.authority; + const authorizedRemoval = currentPolicy !== undefined && nextPolicy === undefined; + const authorizedRestore = currentPolicy === undefined && nextPolicy !== undefined; + if ( + authority?.agentId === normalizedAgentId && + authority.operationId === journal.operation_id && + ((authority.action === "remove" && authorizedRemoval) || + (authority.action === "restore" && authorizedRestore)) + ) { + continue; + } throw new ExecApprovalsMutationFencedError(); } } @@ -144,9 +182,7 @@ export function writeExecApprovalsConfigRow(params: { file: ExecApprovalsFile; raw?: string; now?: number; - leaseOwner?: ExecApprovalsMutationLeaseOwner; }): void { - assertExecApprovalsMutationAllowed({ db: params.db, leaseOwner: params.leaseOwner }); const raw = params.raw ?? serializeExecApprovals(params.file); const values = { config_key: EXEC_APPROVALS_CONFIG_KEY, @@ -176,11 +212,7 @@ export function writeExecApprovalsConfigRow(params: { ); } -export function deleteExecApprovalsConfigRow( - db: DatabaseSync, - leaseOwner?: ExecApprovalsMutationLeaseOwner, -): void { - assertExecApprovalsMutationAllowed({ db, leaseOwner }); +export function deleteExecApprovalsConfigRow(db: DatabaseSync): void { executeSqliteQuerySync( db, getNodeSqliteKysely(db) diff --git a/src/infra/exec-approvals-store.test.ts b/src/infra/exec-approvals-store.test.ts index 40af5e6ba635..e0c6c7ca4237 100644 --- a/src/infra/exec-approvals-store.test.ts +++ b/src/infra/exec-approvals-store.test.ts @@ -76,6 +76,18 @@ function makeStateDatabaseUnavailable(): void { fs.writeFileSync(path.join(stateDir, "state"), "not a directory"); } +const TEST_DELETION_OPERATION_ID = "test-deletion-operation"; + +function seedAgentDeletionJournal(agentId: string, operationId = TEST_DELETION_OPERATION_ID): void { + openOpenClawStateDatabase() + .db.prepare( + `INSERT INTO agent_deletion_journal ( + agent_id, operation_id, agent_dir, workspace_dir, sessions_dir, created_at + ) VALUES (?, ?, '/agent', '/workspace', '/sessions', 1)`, + ) + .run(agentId, operationId); +} + beforeEach(() => { createStateDir(); loggerWarn.mockReset(); @@ -217,6 +229,7 @@ describe("exec approvals SQLite store", () => { kept: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/keep" }] }, }, }); + seedAgentDeletionJournal("removed"); await expect(withAgentExecApprovalsRemoved("removed", async () => "ok")).resolves.toBe("ok"); expect(loadExecApprovals().agents).toEqual({ @@ -234,6 +247,7 @@ describe("exec approvals SQLite store", () => { kept: { security: "deny" }, }, }); + seedAgentDeletionJournal("removed"); let notifyCommitStarted!: () => void; const commitStarted = new Promise((resolve) => { notifyCommitStarted = resolve; @@ -266,8 +280,9 @@ describe("exec approvals SQLite store", () => { await expect(deletion).resolves.toBe("committed"); }); - it("fences writers during deletion even when the agent has no approval policy", async () => { + it("allows unrelated writers while deleting an agent with no approval policy", async () => { saveExecApprovals({ version: 1, agents: { kept: { security: "deny" } } }); + seedAgentDeletionJournal("missing"); let notifyCommitStarted!: () => void; const commitStarted = new Promise((resolve) => { notifyCommitStarted = resolve; @@ -283,48 +298,51 @@ describe("exec approvals SQLite store", () => { await commitStarted; try { - expect(() => - saveExecApprovals({ - version: 1, - agents: { kept: { security: "full" } }, - }), - ).toThrow("Exec approvals cannot be changed while agent deletion is in progress; retry."); + saveExecApprovals({ + version: 1, + agents: { kept: { security: "full" } }, + }); + expect(loadExecApprovals().agents?.kept?.security).toBe("full"); } finally { finishCommit(); } await deletion; }); - it("restores only the removed agent when the surrounding commit fails", async () => { + it("removes and restores every policy alias when the surrounding commit fails", async () => { saveExecApprovals({ version: 1, - agents: { removed: { security: "allowlist" }, kept: { security: "deny" } }, + agents: { + "Agent A": { security: "allowlist" }, + "agent-a": { security: "full" }, + kept: { security: "deny" }, + }, }); + seedAgentDeletionJournal("agent-a"); + let policiesDuringCommit: ReturnType["agents"] = undefined; await expect( - withAgentExecApprovalsRemoved("removed", async () => { + withAgentExecApprovalsRemoved("Agent A", async () => { + policiesDuringCommit = loadExecApprovals().agents; throw new Error("roster commit failed"); }), ).rejects.toThrow("roster commit failed"); - expect(loadExecApprovals().agents).toMatchObject({ - removed: { security: "allowlist" }, + expect(policiesDuringCommit).toEqual({ kept: { security: "deny" } }); + expect(loadExecApprovals().agents).toEqual({ + "Agent A": { security: "allowlist" }, + "agent-a": { security: "full" }, kept: { security: "deny" }, }); }); - it("allows writers after an abandoned mutation lease expires", async () => { - const { db } = openOpenClawStateDatabase(); - const now = Date.now(); - db.prepare( - "INSERT INTO state_leases (scope, lease_key, owner, expires_at, heartbeat_at, payload_json, created_at, updated_at) VALUES (?, ?, ?, ?, ?, NULL, ?, ?)", - ).run("exec-approvals", "mutation", "crashed-deletion", now - 1, now - 10, now - 10, now - 10); + it("requires a deletion journal before commit", async () => { + const commit = vi.fn(async () => "committed"); - await expect( - updateExecApprovals({ - update: () => ({ version: 1, agents: { current: { security: "full" } } }), - }), - ).resolves.toMatchObject({ file: { agents: { current: { security: "full" } } } }); + await expect(withAgentExecApprovalsRemoved("missing", commit)).rejects.toMatchObject({ + name: "ExecApprovalsMutationFencedError", + }); + expect(commit).not.toHaveBeenCalled(); }); it("restores snapshots and honors rollback CAS", async () => { diff --git a/src/infra/exec-approvals-store.ts b/src/infra/exec-approvals-store.ts index 461f7830a159..bd2ac11ca5dd 100644 --- a/src/infra/exec-approvals-store.ts +++ b/src/infra/exec-approvals-store.ts @@ -1,17 +1,15 @@ // Loads, updates, restores, and initializes exec approval policy state. -import { isDeepStrictEqual } from "node:util"; import { AgentDeletionAuthorityRollbackError, AgentDeletionCommitUncertainError, - isAgentDeletionBlocked, } from "../agents/agent-lifecycle-registry.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { normalizeAgentId } from "../routing/session-key.js"; +import { readAgentDeletionJournal } from "../state/agent-deletion-journal.js"; import { openOpenClawStateDatabase, runOpenClawStateWriteTransaction, } from "../state/openclaw-state-db.js"; -import { OpenClawStateLeaseError, withOpenClawStateLease } from "../state/openclaw-state-lease.js"; import { formatErrorMessage } from "./errors.js"; import { createFailClosedExecApprovalsFallback, @@ -27,10 +25,11 @@ import { resetExecApprovalsMigrationGateForTest, } from "./exec-approvals-migration-gate.js"; import { - EXEC_APPROVALS_MUTATION_LEASE_KEY, - EXEC_APPROVALS_MUTATION_LEASE_SCOPE, + assertExecApprovalsMutationAllowed, + assertExecApprovalsMutationAuthority, deleteExecApprovalsConfigRow, - type ExecApprovalsMutationLeaseOwner, + ExecApprovalsMutationFencedError, + type ExecApprovalsMutationAuthority, readExecApprovalsConfigRow, serializeExecApprovals, snapshotFromExecApprovalsRow, @@ -39,8 +38,6 @@ import { const log = createSubsystemLogger("infra/exec-approvals"); const WARN_INTERVAL_MS = 60_000; -const EXEC_APPROVALS_DELETION_LEASE_MS = 120_000; -const EXEC_APPROVALS_DELETION_LEASE_WAIT_MS = 10_000; let lastWarnAt: number | undefined; class ExecApprovalsStoreUnavailableError extends Error { @@ -129,45 +126,9 @@ export function replaceExecApprovalsSnapshot( } type InternalExecApprovalsUpdate = ExecApprovalsUpdate & { - allowDeletedAgentRemoval?: string; - allowDeletedAgentRestore?: string; - leaseOwner?: ExecApprovalsMutationLeaseOwner; + authority?: ExecApprovalsMutationAuthority; }; -function assertNoDeletedAgentApprovalChanged( - current: ExecApprovalsFile, - next: ExecApprovalsFile, - params: Pick< - InternalExecApprovalsUpdate, - "allowDeletedAgentRemoval" | "allowDeletedAgentRestore" - >, -): void { - const agentIds = new Set([ - ...Object.keys(current.agents ?? {}), - ...Object.keys(next.agents ?? {}), - ]); - for (const agentId of agentIds) { - const currentPolicy = current.agents?.[agentId]; - const nextPolicy = next.agents?.[agentId]; - const allowedRemoval = - agentId === params.allowDeletedAgentRemoval && - currentPolicy !== undefined && - nextPolicy === undefined; - const allowedRestore = - agentId === params.allowDeletedAgentRestore && - currentPolicy === undefined && - nextPolicy !== undefined; - if ( - isAgentDeletionBlocked(agentId) && - !allowedRemoval && - !allowedRestore && - !isDeepStrictEqual(currentPolicy, nextPolicy) - ) { - throw new Error(`Exec approvals are unavailable while agent ${agentId} is deleted.`); - } - } -} - function updateExecApprovalsInTransaction( params: InternalExecApprovalsUpdate, ): ExecApprovalsSnapshot | null { @@ -187,12 +148,17 @@ function updateExecApprovalsInTransaction( if (next === null) { return current; } - assertNoDeletedAgentApprovalChanged(current.file, next, params); + assertExecApprovalsMutationAllowed({ + db, + current: current.file, + next, + authority: params.authority, + }); const raw = serializeExecApprovals(next); if (current.exists && current.raw === raw) { return current; } - writeExecApprovalsConfigRow({ db, file: next, raw, leaseOwner: params.leaseOwner }); + writeExecApprovalsConfigRow({ db, file: next, raw }); return snapshotFromExecApprovalsRow({ path: current.path, row: { raw_json: raw }, @@ -217,94 +183,85 @@ export async function updateExecApprovals( return updateExecApprovalsInTransaction(params); } -/** Remove one deleted agent's policy, restoring only that entry if commit fails. */ +/** Remove one deleted agent's policy aliases, restoring them if commit fails. */ export async function withAgentExecApprovalsRemoved( agentId: string, commit: () => Promise, ): Promise { const key = normalizeAgentId(agentId); + const snapshot = readExecApprovalsSnapshot(); + const operationId = readAgentDeletionJournal(key)?.operationId; + if (!operationId) { + throw new ExecApprovalsMutationFencedError(); + } + const removedPolicyEntries = Object.entries(snapshot.file.agents ?? {}).filter( + ([policyKey]) => normalizeAgentId(policyKey) === key, + ); + if (removedPolicyEntries.length > 0) { + const updated = updateExecApprovalsInTransaction({ + baseHash: snapshot.hash, + authority: { action: "remove", agentId: key, operationId }, + update: (file) => { + const agents = { ...file.agents }; + for (const [policyKey] of removedPolicyEntries) { + delete agents[policyKey]; + } + return { ...file, agents }; + }, + }); + if (!updated) { + throw new Error("Exec approvals changed while deleting agent; retry deletion."); + } + } else { + runOpenClawStateWriteTransaction(({ db }) => { + assertExecApprovalsMutationAuthority(db, { + action: "remove", + agentId: key, + operationId, + }); + }); + } try { - return await withOpenClawStateLease( - { - scope: EXEC_APPROVALS_MUTATION_LEASE_SCOPE, - key: EXEC_APPROVALS_MUTATION_LEASE_KEY, - database: { scope: "shared" }, - leaseMs: EXEC_APPROVALS_DELETION_LEASE_MS, - waitMs: EXEC_APPROVALS_DELETION_LEASE_WAIT_MS, - leaseLabel: "exec approvals agent deletion lease", - operationLabel: "exec-approvals.agent-deletion.lease", - }, - async (leaseOwner) => { - // Heartbeats retain the fence during a slow roster commit. If this process - // crashes, the persisted TTL expires and writers can proceed without cleanup. - const snapshot = readExecApprovalsSnapshot(); - const removedPolicy = snapshot.file.agents?.[key]; - if (removedPolicy !== undefined) { - const updated = updateExecApprovalsInTransaction({ - baseHash: snapshot.hash, - allowDeletedAgentRemoval: key, - leaseOwner, - update: (file) => { - const agents = { ...file.agents }; - delete agents[key]; - return { ...file, agents }; - }, - }); - if (!updated) { - throw new Error("Exec approvals changed while deleting agent; retry deletion."); - } - } - try { - return await commit(); - } catch (error) { - if (error instanceof AgentDeletionCommitUncertainError) { - throw error; - } - if (removedPolicy !== undefined) { - try { - updateExecApprovalsInTransaction({ - allowDeletedAgentRestore: key, - leaseOwner, - update: (file) => ({ - ...file, - agents: { ...file.agents, [key]: removedPolicy }, - }), - }); - } catch (rollbackError) { - throw new AgentDeletionAuthorityRollbackError( - [error, rollbackError], - `Failed to roll back exec approvals deletion for agent ${key}.`, - { cause: error }, - ); - } - } - throw error; - } - }, - ); + return await commit(); } catch (error) { - if ( - error instanceof OpenClawStateLeaseError && - error.code === "OPENCLAW_STATE_LEASE_STORAGE_FAILED" - ) { - throw new ExecApprovalsStoreUnavailableError(error); + if (error instanceof AgentDeletionCommitUncertainError) { + throw error; + } + if (removedPolicyEntries.length > 0) { + try { + updateExecApprovalsInTransaction({ + authority: { action: "restore", agentId: key, operationId }, + update: (file) => ({ + ...file, + agents: { ...file.agents, ...Object.fromEntries(removedPolicyEntries) }, + }), + }); + } catch (rollbackError) { + throw new AgentDeletionAuthorityRollbackError( + [error, rollbackError], + `Failed to roll back exec approvals deletion for agent ${key}.`, + { cause: error }, + ); + } } throw error; } } -function restoreExecApprovalsSnapshotInTransaction( - snapshot: ExecApprovalsSnapshot, - leaseOwner?: ExecApprovalsMutationLeaseOwner, -): void { +function restoreExecApprovalsSnapshotInTransaction(snapshot: ExecApprovalsSnapshot): void { runOpenClawStateWriteTransaction( ({ db }) => { + const current = snapshotFromExecApprovalsRow({ + path: resolveExecApprovalsDisplayPath(), + row: readExecApprovalsConfigRow(db), + }); + assertExecApprovalsMutationAllowed({ db, current: current.file, next: snapshot.file }); if (!snapshot.exists) { - deleteExecApprovalsConfigRow(db, leaseOwner); + deleteExecApprovalsConfigRow(db); return; } const raw = snapshot.raw ?? serializeExecApprovals(snapshot.file); - writeExecApprovalsConfigRow({ db, file: snapshot.file, raw, leaseOwner }); + writeExecApprovalsConfigRow({ db, file: snapshot.file, raw }); }, {}, { operationLabel: "exec-approvals.restore" }, @@ -330,6 +287,7 @@ export async function restoreExecApprovalsSnapshotLocked( if (current.hash !== baseHash) { return false; } + assertExecApprovalsMutationAllowed({ db, current: current.file, next: snapshot.file }); if (!snapshot.exists) { deleteExecApprovalsConfigRow(db); } else {