fix(cli): prevent duplicate rendering of auto-approved file tools (#782)

Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
Charles Packer
2026-02-02 00:36:47 -08:00
committed by GitHub
parent 1ee7a13d8b
commit 7297e334f0
4 changed files with 160 additions and 175 deletions

View File

@@ -1575,6 +1575,19 @@ export default function App({
const deferredCommits = deferredToolCallCommitsRef.current;
const now = Date.now();
let blockedByDeferred = false;
// If we eagerly committed a tall preview for file tools, don't also
// commit the successful tool_call line (preview already represents it).
const shouldSkipCommittedToolCall = (ln: Line): boolean => {
if (ln.kind !== "tool_call") return false;
if (!ln.toolCallId || !ln.name) return false;
if (ln.phase !== "finished" || ln.resultOk === false) return false;
if (!eagerCommittedPreviewsRef.current.has(ln.toolCallId)) return false;
return (
isFileEditTool(ln.name) ||
isFileWriteTool(ln.name) ||
isPatchTool(ln.name)
);
};
if (!deferToolCalls && deferredCommits.size > 0) {
deferredCommits.clear();
setDeferredCommitAt(null);
@@ -1646,6 +1659,11 @@ export default function App({
continue;
}
if ("phase" in ln && ln.phase === "finished") {
if (shouldSkipCommittedToolCall(ln)) {
deferredCommits.delete(id);
emittedIdsRef.current.add(id);
continue;
}
if (
deferToolCalls &&
ln.kind === "tool_call" &&
@@ -8361,17 +8379,30 @@ ${SYSTEM_REMINDER_CLOSE}
...(additionalDecision ? [additionalDecision] : []),
];
executingToolCallIdsRef.current = allDecisions
.filter((decision) => decision.type === "approve")
.map((decision) => decision.approval.toolCallId);
const approvedDecisions = allDecisions.filter(
(
decision,
): decision is {
type: "approve";
approval: ApprovalRequest;
precomputedResult?: ToolExecutionResult;
} => decision.type === "approve",
);
const runningDecisions = approvedDecisions.filter(
(decision) => !decision.precomputedResult,
);
executingToolCallIdsRef.current = runningDecisions.map(
(decision) => decision.approval.toolCallId,
);
// Set phase to "running" for all approved tools
setToolCallsRunning(
buffersRef.current,
allDecisions
.filter((d) => d.type === "approve")
.map((d) => d.approval.toolCallId),
);
if (runningDecisions.length > 0) {
setToolCallsRunning(
buffersRef.current,
runningDecisions.map((d) => d.approval.toolCallId),
);
}
refreshDerived();
// Execute approved tools and format results using shared function
@@ -9770,12 +9801,6 @@ ${SYSTEM_REMINDER_CLOSE}
setThinkingMessage(getRandomThinkingVerb());
refreshDerived();
// Mark as eagerly committed to prevent duplicate rendering
// (sendAllResults will call setToolCallsRunning which resets phase to "running")
if (approval.toolCallId) {
eagerCommittedPreviewsRef.current.add(approval.toolCallId);
}
const decision = {
type: "approve" as const,
approval,
@@ -10095,53 +10120,55 @@ Plan file path: ${planFilePath}`;
items={staticItems}
style={{ flexDirection: "column" }}
>
{(item: StaticItem, index: number) => (
<Box key={item.id} marginTop={index > 0 ? 1 : 0}>
{item.kind === "welcome" ? (
<WelcomeScreen loadingState="ready" {...item.snapshot} />
) : item.kind === "user" ? (
<UserMessage line={item} />
) : item.kind === "reasoning" ? (
<ReasoningMessage line={item} />
) : item.kind === "assistant" ? (
<AssistantMessage line={item} />
) : item.kind === "tool_call" ? (
<ToolCallMessage
line={item}
precomputedDiffs={precomputedDiffsRef.current}
lastPlanFilePath={lastPlanFilePathRef.current}
/>
) : item.kind === "subagent_group" ? (
<SubagentGroupStatic agents={item.agents} />
) : item.kind === "error" ? (
<ErrorMessage line={item} />
) : item.kind === "status" ? (
<StatusMessage line={item} />
) : item.kind === "event" ? (
<EventMessage line={item} />
) : item.kind === "separator" ? (
<Box marginTop={1}>
<Text dimColor>{"─".repeat(columns)}</Text>
</Box>
) : item.kind === "command" ? (
<CommandMessage line={item} />
) : item.kind === "bash_command" ? (
<BashCommandMessage line={item} />
) : item.kind === "trajectory_summary" ? (
<TrajectorySummary line={item} />
) : item.kind === "approval_preview" ? (
<ApprovalPreview
toolName={item.toolName}
toolArgs={item.toolArgs}
precomputedDiff={item.precomputedDiff}
allDiffs={precomputedDiffsRef.current}
planContent={item.planContent}
planFilePath={item.planFilePath}
toolCallId={item.toolCallId}
/>
) : null}
</Box>
)}
{(item: StaticItem, index: number) => {
return (
<Box key={item.id} marginTop={index > 0 ? 1 : 0}>
{item.kind === "welcome" ? (
<WelcomeScreen loadingState="ready" {...item.snapshot} />
) : item.kind === "user" ? (
<UserMessage line={item} />
) : item.kind === "reasoning" ? (
<ReasoningMessage line={item} />
) : item.kind === "assistant" ? (
<AssistantMessage line={item} />
) : item.kind === "tool_call" ? (
<ToolCallMessage
line={item}
precomputedDiffs={precomputedDiffsRef.current}
lastPlanFilePath={lastPlanFilePathRef.current}
/>
) : item.kind === "subagent_group" ? (
<SubagentGroupStatic agents={item.agents} />
) : item.kind === "error" ? (
<ErrorMessage line={item} />
) : item.kind === "status" ? (
<StatusMessage line={item} />
) : item.kind === "event" ? (
<EventMessage line={item} />
) : item.kind === "separator" ? (
<Box marginTop={1}>
<Text dimColor>{"─".repeat(columns)}</Text>
</Box>
) : item.kind === "command" ? (
<CommandMessage line={item} />
) : item.kind === "bash_command" ? (
<BashCommandMessage line={item} />
) : item.kind === "trajectory_summary" ? (
<TrajectorySummary line={item} />
) : item.kind === "approval_preview" ? (
<ApprovalPreview
toolName={item.toolName}
toolArgs={item.toolArgs}
precomputedDiff={item.precomputedDiff}
allDiffs={precomputedDiffsRef.current}
planContent={item.planContent}
planFilePath={item.planFilePath}
toolCallId={item.toolCallId}
/>
) : null}
</Box>
);
}}
</Static>
<Box flexDirection="column">
@@ -10162,6 +10189,21 @@ Plan file path: ${planFilePath}`;
{liveItems.length > 0 && (
<Box flexDirection="column">
{liveItems.map((ln) => {
const isFileTool =
ln.kind === "tool_call" &&
ln.name &&
(isFileEditTool(ln.name) ||
isFileWriteTool(ln.name) ||
isPatchTool(ln.name));
const isApprovalTracked =
ln.kind === "tool_call" &&
ln.toolCallId &&
(ln.toolCallId === currentApproval?.toolCallId ||
pendingIds.has(ln.toolCallId) ||
queuedIds.has(ln.toolCallId));
if (isFileTool && !isApprovalTracked) {
return null;
}
// Skip Task tools that don't have a pending approval
// They render as empty Boxes (ToolCallMessage returns null for non-finished Task tools)
// which causes N blank lines when N Task tools are called in parallel
@@ -10178,18 +10220,6 @@ Plan file path: ${planFilePath}`;
return null;
}
// Skip tool calls that were eagerly committed to staticItems
// (e.g., ExitPlanMode preview) - but only AFTER approval is complete
// We still need to render the approval options while awaiting approval
if (
ln.kind === "tool_call" &&
ln.toolCallId &&
eagerCommittedPreviewsRef.current.has(ln.toolCallId) &&
ln.toolCallId !== currentApproval?.toolCallId
) {
return null;
}
// Check if this tool call matches the current approval awaiting user input
const matchesCurrentApproval =
ln.kind === "tool_call" &&

View File

@@ -314,8 +314,7 @@ export function markCurrentLineAsFinished(b: Buffers) {
// console.log(`[MARK_CURRENT_FINISHED] No lastOtid, returning`);
return;
}
// Try both the plain otid and the -tool suffix (in case of collision workaround)
const prev = b.byId.get(b.lastOtid) || b.byId.get(`${b.lastOtid}-tool`);
const prev = b.byId.get(b.lastOtid);
// console.log(`[MARK_CURRENT_FINISHED] Found line: kind=${prev?.kind}, phase=${(prev as any)?.phase}`);
if (prev && (prev.kind === "assistant" || prev.kind === "reasoning")) {
// console.log(`[MARK_CURRENT_FINISHED] Marking ${b.lastOtid} as finished`);
@@ -386,6 +385,7 @@ function getStringProp(obj: Record<string, unknown>, key: string) {
const v = obj[key];
return typeof v === "string" ? v : undefined;
}
function extractTextPart(v: unknown): string {
if (typeof v === "string") return v;
if (Array.isArray(v)) {
@@ -562,25 +562,8 @@ export function onChunk(b: Buffers, chunk: LettaStreamingResponse) {
case "tool_call_message":
case "approval_request_message": {
/* POST-FIX VERSION (what this should look like after backend fix):
const id = chunk.otid;
// Handle otid transition (mark previous line as finished)
handleOtidTransition(b, id);
if (!id) break;
const toolCall = chunk.tool_call || (Array.isArray(chunk.tool_calls) && chunk.tool_calls.length > 0 ? chunk.tool_calls[0] : null);
const toolCallId = toolCall?.tool_call_id;
const name = toolCall?.name;
const argsText = toolCall?.arguments;
// Record correlation: toolCallId → line id (otid)
if (toolCallId) b.toolCallIdToLineId.set(toolCallId, id);
*/
let id = chunk.otid;
// console.log(`[TOOL_CALL] Received ${chunk.message_type} with otid=${id}, toolCallId=${chunk.tool_call?.tool_call_id}, name=${chunk.tool_call?.name}`);
handleOtidTransition(b, chunk.otid ?? undefined);
// Use deprecated tool_call or new tool_calls array
const toolCall =
@@ -588,74 +571,38 @@ export function onChunk(b: Buffers, chunk: LettaStreamingResponse) {
(Array.isArray(chunk.tool_calls) && chunk.tool_calls.length > 0
? chunk.tool_calls[0]
: null);
if (!toolCall || !toolCall.tool_call_id) break;
const toolCallId = toolCall?.tool_call_id;
const name = toolCall?.name;
const argsText = toolCall?.arguments;
const toolCallId = toolCall.tool_call_id;
const name = toolCall.name;
const argsText = toolCall.arguments;
// ========== START BACKEND BUG WORKAROUND (Remove after OTID fix) ==========
// Bug: Backend sends same otid for reasoning and tool_call, and multiple otids for same tool_call
// Check if we already have a line for this toolCallId (prevents duplicates)
if (toolCallId && b.toolCallIdToLineId.has(toolCallId)) {
// Update the existing line instead of creating a new one
const existingId = b.toolCallIdToLineId.get(toolCallId);
if (existingId) {
id = existingId;
}
// Handle otid transition for tracking purposes
handleOtidTransition(b, chunk.otid ?? undefined);
} else {
// Check if this otid is already used by another line
if (id && b.byId.has(id)) {
const existing = b.byId.get(id);
if (existing && existing.kind === "reasoning") {
// Mark the reasoning as finished before we create the tool_call
markAsFinished(b, id);
// Use a different ID for the tool_call to avoid overwriting the reasoning
id = `${id}-tool`;
} else if (existing && existing.kind === "tool_call") {
// Parallel tool calls: same otid, different tool_call_id
// Create unique ID for this parallel tool using its tool_call_id
if (toolCallId) {
id = `${id}-${toolCallId.slice(-8)}`;
} else {
// Fallback: append timestamp
id = `${id}-${Date.now().toString(36)}`;
}
}
}
// ========== END BACKEND BUG WORKAROUND ==========
// This part stays after fix:
// Handle otid transition (mark previous line as finished)
// This must happen BEFORE the break, so reasoning gets finished even when tool has no otid
handleOtidTransition(b, id ?? undefined);
if (!id) {
// console.log(`[TOOL_CALL] No otid, breaking`);
break;
}
// Record correlation: toolCallId → line id (otid) for future updates
if (toolCallId) b.toolCallIdToLineId.set(toolCallId, id);
// Use tool_call_id as the stable line id (server guarantees uniqueness).
const id = b.toolCallIdToLineId.get(toolCallId) ?? toolCallId;
if (!b.toolCallIdToLineId.has(toolCallId)) {
b.toolCallIdToLineId.set(toolCallId, id);
}
// Early exit if no valid id
if (!id) break;
// Tool calls should be "ready" (blinking) while pending execution
// Only approval requests explicitly set to "ready", but regular tool calls should also blink
const desiredPhase = "ready";
const line = ensure<ToolCallLine>(b, id, () => ({
let line = ensure<ToolCallLine>(b, id, () => ({
kind: "tool_call",
id,
toolCallId: toolCallId ?? undefined,
toolCallId,
name: name ?? undefined,
phase: desiredPhase,
}));
// If additional metadata arrives later (e.g., name), update the line.
if ((name && !line.name) || line.toolCallId !== toolCallId) {
line = {
...line,
toolCallId,
name: line.name ?? name ?? undefined,
};
b.byId.set(id, line);
}
// If this is an approval request and the line already exists, bump phase to ready
if (
chunk.message_type === "approval_request_message" &&

View File

@@ -41,9 +41,6 @@ export class StreamProcessor {
public lastSeqId: number | null = null;
public stopReason: StopReasonType | null = null;
// Approval ID fallback (for backends that don't include tool_call_id in every chunk)
private lastApprovalId: string | null = null;
processChunk(chunk: LettaStreamingResponse): ChunkProcessingResult {
let errorInfo: ErrorInfo | undefined;
let updatedApproval: ApprovalRequest | undefined;
@@ -110,11 +107,6 @@ export class StreamProcessor {
// Continue processing this chunk (for UI display)
}
// Need to store the approval request ID to send an approval in a new run
if (chunk.message_type === "approval_request_message") {
this.lastApprovalId = chunk.id;
}
// Accumulate approval request state across streaming chunks
// Support parallel tool calls by tracking each tool_call_id separately
// NOTE: Only track approval_request_message, NOT tool_call_message
@@ -134,21 +126,9 @@ export class StreamProcessor {
: [];
for (const toolCall of toolCalls) {
// Many backends stream tool_call chunks where only the first frame
// carries the tool_call_id; subsequent argument deltas omit it.
// Fall back to the last seen id within this turn so we can
// properly accumulate args.
let id: string | null = toolCall?.tool_call_id ?? this.lastApprovalId;
if (!id) {
// As an additional guard, if exactly one approval is being
// tracked already, use that id for continued argument deltas.
if (this.pendingApprovals.size === 1) {
id = Array.from(this.pendingApprovals.keys())[0] ?? null;
}
}
if (!id) continue; // cannot safely attribute this chunk
this.lastApprovalId = id;
const toolCallId = toolCall?.tool_call_id;
if (!toolCallId) continue; // contract: approval chunks include tool_call_id
const id = toolCallId;
// Get or create entry for this tool_call_id
const existing = this.pendingApprovals.get(id) || {

View File

@@ -1,5 +1,9 @@
// src/utils/debug.ts
// Simple debug logging utility - only logs when LETTA_DEBUG env var is set
// Optionally logs to a file when LETTA_DEBUG_FILE is set
import { appendFileSync } from "node:fs";
import { format } from "node:util";
/**
* Check if debug mode is enabled via LETTA_DEBUG env var
@@ -10,6 +14,30 @@ export function isDebugEnabled(): boolean {
return debug === "1" || debug === "true";
}
function getDebugFile(): string | null {
const path = process.env.LETTA_DEBUG_FILE;
return path && path.trim().length > 0 ? path : null;
}
function writeDebugLine(
prefix: string,
message: string,
args: unknown[],
): void {
const debugFile = getDebugFile();
const line = `${format(`[${prefix}] ${message}`, ...args)}\n`;
if (debugFile) {
try {
appendFileSync(debugFile, line, { encoding: "utf8" });
return;
} catch {
// Fall back to console if file write fails
}
}
// Default to console output
console.log(line.trimEnd());
}
/**
* Log a debug message (only if LETTA_DEBUG is enabled)
* @param prefix - A prefix/tag for the log message (e.g., "check-approval")
@@ -22,7 +50,7 @@ export function debugLog(
...args: unknown[]
): void {
if (isDebugEnabled()) {
console.log(`[${prefix}] ${message}`, ...args);
writeDebugLine(prefix, message, args);
}
}
@@ -38,6 +66,6 @@ export function debugWarn(
...args: unknown[]
): void {
if (isDebugEnabled()) {
console.warn(`[${prefix}] ${message}`, ...args);
writeDebugLine(prefix, `WARN: ${message}`, args);
}
}