fix(chat): keep desktop tools running in chats left mid-turn - #8311
waleedlatif1 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- In
apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts, a terminal call first seen more than two minutes after emission can be silently dropped, permanently stalling the turn; explicitly settle stale calls as failed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts:104">
P1: A detached relay can permanently stall a turn when it first sees a terminal call more than two minutes after emission. The terminal executor drops stale events without reporting a failure, so settle stale calls with an error (as the browser executor does) instead of returning silently.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
c938242 to
b60c919
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 3/5
- In
apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts, organization-owned detached chats can have user-local filesystem tool events consumed without starting the tool or reporting an error; provide the requiredworkspaceIdor handle the event explicitly. - In
apps/sim/lib/mothership/tools/client/local-filesystem.ts, the LRU can evict an in-flight call ID, allowing a replay to trigger duplicate bridge work; keep active IDs pinned until completion.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/mothership/tools/client/local-filesystem.ts">
<violation number="1" location="apps/sim/lib/mothership/tools/client/local-filesystem.ts:346">
P2: This finite LRU can evict a local-filesystem call while it is still running, defeating the exactly-once guarantee and causing duplicate bridge work when the detached relay or remounted view replays it. Keep active IDs non-evictable, or make eviction completion-aware instead of assuming every evicted call has settled.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts:106">
P1: Organization-owned detached chats pass no `workspaceId`, but `launchLocalFilesystemTool` refuses user-local filesystem calls without one. The relay then consumes the tool event without starting it or reporting an error, leaving the server run stalled; allow user-local execution without a workspace context or explicitly settle unsupported calls.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| return | ||
| case 'localFilesystem': | ||
| launchLocalFilesystemTool(toolCallId, toolName, args, { | ||
| workspaceId: turn.workspaceId, |
There was a problem hiding this comment.
P1: Organization-owned detached chats pass no workspaceId, but launchLocalFilesystemTool refuses user-local filesystem calls without one. The relay then consumes the tool event without starting it or reporting an error, leaving the server run stalled; allow user-local execution without a workspace context or explicitly settle unsupported calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/workspace/[workspaceId]/home/hooks/stream/detached-client-tools.ts, line 106:
<comment>Organization-owned detached chats pass no `workspaceId`, but `launchLocalFilesystemTool` refuses user-local filesystem calls without one. The relay then consumes the tool event without starting it or reporting an error, leaving the server run stalled; allow user-local execution without a workspace context or explicitly settle unsupported calls.</comment>
<file context>
@@ -0,0 +1,243 @@
+ return
+ case 'localFilesystem':
+ launchLocalFilesystemTool(toolCallId, toolName, args, {
+ workspaceId: turn.workspaceId,
+ chatId,
+ })
</file context>
| * user leaves can both start it, and a remounted view replays calls still | ||
| * running. Bounded: a call evicted behind this many newer ones is long settled. | ||
| */ | ||
| const executedToolCallIds = new LRUCache<string, true>({ max: 500 }) |
There was a problem hiding this comment.
P2: This finite LRU can evict a local-filesystem call while it is still running, defeating the exactly-once guarantee and causing duplicate bridge work when the detached relay or remounted view replays it. Keep active IDs non-evictable, or make eviction completion-aware instead of assuming every evicted call has settled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/mothership/tools/client/local-filesystem.ts, line 346:
<comment>This finite LRU can evict a local-filesystem call while it is still running, defeating the exactly-once guarantee and causing duplicate bridge work when the detached relay or remounted view replays it. Keep active IDs non-evictable, or make eviction completion-aware instead of assuming every evicted call has settled.</comment>
<file context>
@@ -337,12 +338,21 @@ async function execute(
+ * user leaves can both start it, and a remounted view replays calls still
+ * running. Bounded: a call evicted behind this many newer ones is long settled.
+ */
+const executedToolCallIds = new LRUCache<string, true>({ max: 500 })
+
export function executeLocalFilesystemTool(
</file context>
| void reportClientToolCompletion( | ||
| toolCallId, | ||
| ASYNC_TOOL_CONFIRMATION_STATUS.error, | ||
| STALE_EVENT_MESSAGE, | ||
| { error: STALE_EVENT_MESSAGE, staleEvent: true } |
There was a problem hiding this comment.
| } | ||
|
|
||
| import('@/lib/mothership/tools/client/local-filesystem').then( | ||
| (m) => m.executeLocalFilesystemTool(toolCallId, toolName, args, context), |
There was a problem hiding this comment.
This branch was previously deployed
1 participant
Summary
detached-client-tools.ts). It reads the stream from where the view stopped and starts each call until the run ends or the chat's view reads the stream again<Home key={chatId}>)resolveClientToolStart) so the view and the relay classify calls identically. It replaces four copy-pasted blocks inhandleToolEvent, with no behavior changeLRUCacheguard in its lazily loaded executor, matching the browser and terminal executors. The terminal executor now reports stale calls it never ran instead of dropping them silentlyType of Change
Testing
Checklist
test-auditauthoring gate)