fix: resolve approval desync issues for slash commands and queued messages (#477)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
221
src/cli/App.tsx
221
src/cli/App.tsx
@@ -1851,8 +1851,12 @@ export default function App({
|
||||
|
||||
// Unexpected stop reason (error, llm_api_error, etc.)
|
||||
// Cache desync detection and last failure for consistent handling
|
||||
const isApprovalPayload =
|
||||
currentInput.length === 1 && currentInput[0]?.type === "approval";
|
||||
// Check if payload contains approvals (could be approval-only or mixed with user message)
|
||||
const hasApprovalInPayload = currentInput.some(
|
||||
(item) => item?.type === "approval",
|
||||
);
|
||||
const isApprovalOnlyPayload =
|
||||
hasApprovalInPayload && currentInput.length === 1;
|
||||
|
||||
// Capture the most recent error text in this turn (if any)
|
||||
let latestErrorText: string | null = null;
|
||||
@@ -1876,7 +1880,8 @@ export default function App({
|
||||
const lastFailureMessage = latestErrorText || detailFromRun || null;
|
||||
|
||||
// Check for approval desync errors even if stop_reason isn't llm_api_error.
|
||||
if (isApprovalPayload && desyncDetected) {
|
||||
// Handle both approval-only payloads and mixed [approval, message] payloads.
|
||||
if (hasApprovalInPayload && desyncDetected) {
|
||||
if (llmApiErrorRetriesRef.current < LLM_API_ERROR_MAX_RETRIES) {
|
||||
llmApiErrorRetriesRef.current += 1;
|
||||
const statusId = uid("status");
|
||||
@@ -1890,11 +1895,29 @@ export default function App({
|
||||
buffersRef.current.order.push(statusId);
|
||||
refreshDerived();
|
||||
|
||||
currentInput.splice(
|
||||
0,
|
||||
currentInput.length,
|
||||
buildApprovalRecoveryMessage(),
|
||||
);
|
||||
if (isApprovalOnlyPayload) {
|
||||
// Approval-only payload: send recovery prompt
|
||||
currentInput.splice(
|
||||
0,
|
||||
currentInput.length,
|
||||
buildApprovalRecoveryMessage(),
|
||||
);
|
||||
} else {
|
||||
// Mixed payload [approval, message]: strip stale approval, keep user message
|
||||
const messageItems = currentInput.filter(
|
||||
(item) => item?.type !== "approval",
|
||||
);
|
||||
if (messageItems.length > 0) {
|
||||
currentInput.splice(0, currentInput.length, ...messageItems);
|
||||
} else {
|
||||
// Fallback if somehow no message items remain
|
||||
currentInput.splice(
|
||||
0,
|
||||
currentInput.length,
|
||||
buildApprovalRecoveryMessage(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Remove the transient status before retrying
|
||||
buffersRef.current.byId.delete(statusId);
|
||||
@@ -2556,6 +2579,154 @@ export default function App({
|
||||
[refreshDerived],
|
||||
);
|
||||
|
||||
/**
|
||||
* Check and handle any pending approvals before sending a slash command.
|
||||
* Returns true if approvals need user input (caller should return { submitted: false }).
|
||||
* Returns false if no approvals or all auto-handled (caller can proceed).
|
||||
*/
|
||||
const checkPendingApprovalsForSlashCommand = useCallback(async (): Promise<
|
||||
{ blocked: true } | { blocked: false }
|
||||
> => {
|
||||
if (!CHECK_PENDING_APPROVALS_BEFORE_SEND) {
|
||||
return { blocked: false };
|
||||
}
|
||||
|
||||
try {
|
||||
const client = await getClient();
|
||||
const agent = await client.agents.retrieve(agentId);
|
||||
const { pendingApprovals: existingApprovals } = await getResumeData(
|
||||
client,
|
||||
agent,
|
||||
);
|
||||
|
||||
if (!existingApprovals || existingApprovals.length === 0) {
|
||||
return { blocked: false };
|
||||
}
|
||||
|
||||
// There are pending approvals - check permissions (respects yolo mode)
|
||||
const approvalResults = await Promise.all(
|
||||
existingApprovals.map(async (approvalItem) => {
|
||||
if (!approvalItem.toolName) {
|
||||
return {
|
||||
approval: approvalItem,
|
||||
permission: {
|
||||
decision: "deny" as const,
|
||||
reason: "Tool call incomplete - missing name",
|
||||
},
|
||||
context: null,
|
||||
};
|
||||
}
|
||||
const parsedArgs = safeJsonParseOr<Record<string, unknown>>(
|
||||
approvalItem.toolArgs,
|
||||
{},
|
||||
);
|
||||
const permission = await checkToolPermission(
|
||||
approvalItem.toolName,
|
||||
parsedArgs,
|
||||
);
|
||||
const context = await analyzeToolApproval(
|
||||
approvalItem.toolName,
|
||||
parsedArgs,
|
||||
);
|
||||
return { approval: approvalItem, permission, context };
|
||||
}),
|
||||
);
|
||||
|
||||
// Categorize by permission decision
|
||||
const needsUserInput: typeof approvalResults = [];
|
||||
const autoAllowed: typeof approvalResults = [];
|
||||
const autoDenied: typeof approvalResults = [];
|
||||
|
||||
for (const ac of approvalResults) {
|
||||
const { approval, permission } = ac;
|
||||
let decision = permission.decision;
|
||||
|
||||
if (
|
||||
alwaysRequiresUserInput(approval.toolName) &&
|
||||
decision === "allow"
|
||||
) {
|
||||
decision = "ask";
|
||||
}
|
||||
|
||||
if (decision === "ask") {
|
||||
needsUserInput.push(ac);
|
||||
} else if (decision === "deny") {
|
||||
autoDenied.push(ac);
|
||||
} else {
|
||||
autoAllowed.push(ac);
|
||||
}
|
||||
}
|
||||
|
||||
// If any approvals need user input, show dialog
|
||||
if (needsUserInput.length > 0) {
|
||||
setPendingApprovals(needsUserInput.map((ac) => ac.approval));
|
||||
setApprovalContexts(
|
||||
needsUserInput
|
||||
.map((ac) => ac.context)
|
||||
.filter((ctx): ctx is ApprovalContext => ctx !== null),
|
||||
);
|
||||
return { blocked: true };
|
||||
}
|
||||
|
||||
// All approvals can be auto-handled - execute them before proceeding
|
||||
const allResults: ApprovalResult[] = [];
|
||||
|
||||
// Execute auto-allowed tools
|
||||
if (autoAllowed.length > 0) {
|
||||
const autoAllowedResults = await executeAutoAllowedTools(
|
||||
autoAllowed,
|
||||
(chunk) => onChunk(buffersRef.current, chunk),
|
||||
);
|
||||
// Map to ApprovalResult format (ToolReturn)
|
||||
allResults.push(
|
||||
...autoAllowedResults.map((ar) => ({
|
||||
type: "tool" as const,
|
||||
tool_call_id: ar.toolCallId,
|
||||
tool_return: ar.result.toolReturn,
|
||||
status: ar.result.status,
|
||||
stdout: ar.result.stdout,
|
||||
stderr: ar.result.stderr,
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
// Create denial results for auto-denied
|
||||
for (const ac of autoDenied) {
|
||||
const reason = ac.permission.reason || "Permission denied";
|
||||
// Update UI with denial
|
||||
onChunk(buffersRef.current, {
|
||||
message_type: "tool_return_message",
|
||||
id: "dummy",
|
||||
date: new Date().toISOString(),
|
||||
tool_call_id: ac.approval.toolCallId,
|
||||
tool_return: `Error: request to call tool denied. User reason: ${reason}`,
|
||||
status: "error",
|
||||
stdout: null,
|
||||
stderr: null,
|
||||
});
|
||||
// Map to ApprovalResult format (ApprovalReturn)
|
||||
allResults.push({
|
||||
type: "approval" as const,
|
||||
tool_call_id: ac.approval.toolCallId,
|
||||
approve: false,
|
||||
reason,
|
||||
});
|
||||
}
|
||||
|
||||
// Send all results to server if any
|
||||
if (allResults.length > 0) {
|
||||
await processConversation([
|
||||
{ type: "approval", approvals: allResults },
|
||||
]);
|
||||
}
|
||||
|
||||
return { blocked: false };
|
||||
} catch {
|
||||
// If check fails, proceed anyway (don't block user)
|
||||
return { blocked: false };
|
||||
}
|
||||
}, [agentId, processConversation]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: refs read .current dynamically, complex callback with intentional deps
|
||||
const onSubmit = useCallback(
|
||||
async (message?: string): Promise<{ submitted: boolean }> => {
|
||||
@@ -2625,6 +2796,12 @@ export default function App({
|
||||
waitingForQueueCancelRef.current = true;
|
||||
queueSnapshotRef.current = [...newQueue];
|
||||
|
||||
// Abort client-side tool execution if in progress
|
||||
// This makes tool interruption visible immediately instead of waiting for completion
|
||||
if (toolAbortControllerRef.current) {
|
||||
toolAbortControllerRef.current.abort();
|
||||
}
|
||||
|
||||
// Send cancel request to backend (fire-and-forget)
|
||||
getClient()
|
||||
.then((client) => client.agents.messages.cancel(agentId))
|
||||
@@ -3551,6 +3728,12 @@ export default function App({
|
||||
|
||||
// Special handling for /skill command - enter skill creation mode
|
||||
if (trimmed.startsWith("/skill")) {
|
||||
// Check for pending approvals before sending
|
||||
const approvalCheck = await checkPendingApprovalsForSlashCommand();
|
||||
if (approvalCheck.blocked) {
|
||||
return { submitted: false }; // Keep /skill in input box, user handles approval first
|
||||
}
|
||||
|
||||
const cmdId = uid("cmd");
|
||||
|
||||
// Extract optional description after `/skill`
|
||||
@@ -3626,6 +3809,12 @@ export default function App({
|
||||
|
||||
// Special handling for /remember command - remember something from conversation
|
||||
if (trimmed.startsWith("/remember")) {
|
||||
// Check for pending approvals before sending (mirrors regular message flow)
|
||||
const approvalCheck = await checkPendingApprovalsForSlashCommand();
|
||||
if (approvalCheck.blocked) {
|
||||
return { submitted: false }; // Keep /remember in input box, user handles approval first
|
||||
}
|
||||
|
||||
const cmdId = uid("cmd");
|
||||
|
||||
// Extract optional description after `/remember`
|
||||
@@ -3700,6 +3889,12 @@ export default function App({
|
||||
|
||||
// Special handling for /init command - initialize agent memory
|
||||
if (trimmed === "/init") {
|
||||
// Check for pending approvals before sending
|
||||
const approvalCheck = await checkPendingApprovalsForSlashCommand();
|
||||
if (approvalCheck.blocked) {
|
||||
return { submitted: false }; // Keep /init in input box, user handles approval first
|
||||
}
|
||||
|
||||
const cmdId = uid("cmd");
|
||||
buffersRef.current.byId.set(cmdId, {
|
||||
kind: "command",
|
||||
@@ -3846,6 +4041,12 @@ ${gitContext}
|
||||
const matchedCustom = await findCustomCommand(commandName);
|
||||
|
||||
if (matchedCustom) {
|
||||
// Check for pending approvals before sending
|
||||
const approvalCheck = await checkPendingApprovalsForSlashCommand();
|
||||
if (approvalCheck.blocked) {
|
||||
return { submitted: false }; // Keep custom command in input box, user handles approval first
|
||||
}
|
||||
|
||||
const cmdId = uid("cmd");
|
||||
|
||||
// Extract arguments (everything after command name)
|
||||
@@ -4635,6 +4836,10 @@ DO NOT respond to these messages or otherwise consider them in your response unl
|
||||
setQueuedApprovalResults(allResults as ApprovalResult[]);
|
||||
}
|
||||
setStreaming(false);
|
||||
|
||||
// Reset queue-cancel flag so dequeue effect can fire
|
||||
waitingForQueueCancelRef.current = false;
|
||||
queueSnapshotRef.current = [];
|
||||
} else {
|
||||
// Continue conversation with all results
|
||||
await processConversation([
|
||||
|
||||
@@ -193,6 +193,18 @@ export async function drainStream(
|
||||
break;
|
||||
}
|
||||
|
||||
// Suppress mid-stream desync errors (match headless behavior)
|
||||
// These are transient and will be handled by end-of-turn desync recovery
|
||||
const errObj = (chunk as unknown as { error?: { detail?: string } })
|
||||
.error;
|
||||
if (
|
||||
errObj?.detail?.includes("No tool call is currently awaiting approval")
|
||||
) {
|
||||
// Server isn't ready for approval yet; let the stream continue
|
||||
// Suppress the error frame from output
|
||||
continue;
|
||||
}
|
||||
|
||||
onChunk(buffers, chunk);
|
||||
queueMicrotask(refresh);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user