fix(mothership): fix boundary

[Sg312]

Summary

Fix boundary issues

Type of Change

• Bug fix

Testing

Manual

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Mar 24, 2026 10:14pm

[greptile-apps]

Greptile Summary

This PR replaces the client-side async tool resumption logic (claim/release/deliver flow against the local DB) with a lighter Go-side checkpoint polling model: Sim now polls /api/tools/checkpoint-status until the Go backend reports readiness, then issues a resume request containing only the checkpointId. It also changes markToolComplete from fire-and-forget to fully awaited, ensuring proper ordering before resource persistence and improving error visibility.

• index.ts: Removes claimCompletedAsyncToolCall, releaseCompletedAsyncToolClaim, markAsyncToolDelivered, and didAsyncToolSucceed in favour of waitForCheckpointReady (40-attempt exponential poll). Adds upstream retry for 502/503/504 on the resume route. Two dead-code artifacts left behind: let resumeReady = false (line 279) and if (!resumeReady) { break } (lines 340–342) can be removed.
• tool-execution.ts: markToolComplete is now awaited via waitForMarkComplete (previously fire-and-forget). reportCancelledTool is now async. Added ASYNC_RESUME_DIAG_TAG diagnostic logging at each completion phase.
• handlers.ts: Added diagnostic logging before context.awaitingAsyncContinuation is set in the done handler.
• Tests: Updated mocks to reflect the new boundary; fetchMock stubs the checkpoint-status endpoint; all four continuation scenarios are covered including the 502 upstream retry path.

Confidence Score: 5/5

• Safe to merge; the only finding is cosmetic dead code that poses no runtime risk.
• The architectural change is well-scoped and thoroughly tested. All four async-continuation scenarios are covered in the updated test suite, including the new checkpoint polling and upstream retry paths. The markToolComplete promotion from fire-and-forget to awaited is coherent with the new boundary design (tool execution is always a background promise, so the SSE reader remains unblocked). The sole issue is the leftover resumeReady variable and its dead guard — a non-blocking cleanup with no impact on correctness.
• No files require special attention beyond the minor dead-code cleanup in apps/sim/lib/copilot/orchestrator/index.ts.

Important Files Changed

Filename	Overview
apps/sim/lib/copilot/orchestrator/index.ts	Core change: replaces claim-based async tool resumption with Go-side checkpoint polling. New waitForCheckpointReady polls /api/tools/checkpoint-status with exponential backoff (up to 40 × 250ms). Upstream resume retries added (3× for 502/503/504). Dead code: resumeReady variable and its guard are leftover from the old inner loop and should be removed.
apps/sim/lib/copilot/orchestrator/sse/handlers/tool-execution.ts	Changed markToolComplete from fire-and-forget to fully awaited (via new waitForMarkComplete wrapper). reportCancelledTool is now async and awaited. Added diagnostic logging at each completion phase. The old "must not await to avoid deadlock" comment is gone, consistent with the new boundary design where tool execution is always a background promise.
apps/sim/lib/copilot/orchestrator/sse/handlers/handlers.ts	Added ASYNC_RESUME_DIAG_TAG constant and diagnostic logger.warn call in the done handler before setting context.awaitingAsyncContinuation, improving observability of the async-pause flow.
apps/sim/lib/copilot/orchestrator/index.test.ts	Tests updated to reflect new checkpoint polling model. Mocks trimmed to only relevant functions. New fetchMock stubs the checkpoint-status endpoint. afterEach properly restores fake timers and globals. All four scenarios covered: readiness success, readiness failure, async-pause done event, and upstream 502 retry.
apps/sim/lib/copilot/orchestrator/sse/handlers/handlers.test.ts	Updated test mock setup to include upsertAsyncToolCall. Tests cover tool execution, deduplication, cancellation, in-flight promise stability, mark-complete ordering, and upsert failure resilience. Coverage looks solid.

Sequence Diagram

sequenceDiagram
    participant Sim as Sim (orchestrator)
    participant Go as Go Backend

    Sim->>Go: POST /api/copilot (initial stream)
    Go-->>Sim: SSE events (tool_call, content, ...)
    Note over Sim: tool_call handler fires executeToolAndReport() in background
    Go-->>Sim: SSE done {async_pause: {checkpointId, pendingToolCallIds}}
    Note over Sim: context.awaitingAsyncContinuation set

    loop Poll checkpoint readiness (up to 40x × 250ms backoff)
        Sim->>Go: GET /api/tools/checkpoint-status?checkpointId=...
        Go-->>Sim: {ready: false}
    end
    Sim->>Go: GET /api/tools/checkpoint-status?checkpointId=...
    Go-->>Sim: {ready: true}

    loop Retry on 502/503/504 (up to 3x)
        Sim->>Go: POST /api/tools/resume {checkpointId}
        Go-->>Sim: SSE events (next turn)
    end

    Note over Sim: executeToolAndReport awaits markToolComplete (no longer fire-and-forget)
    Sim->>Go: markToolComplete(toolCallId, status, result)
    Go-->>Sim: ack

Loading

_{Reviews (1): Last reviewed commit: "Fix lint" | Re-trigger Greptile}

[greptile-apps]

Dead code: resumeReady flag is always true here

resumeReady is set to true unconditionally on line 338, immediately before this guard. There is no code path that reaches if (!resumeReady) with resumeReady === false — if checkpoint readiness failed, the earlier if (!readiness.ready) block pushes an error and breaks out of the loop before getting here. This guard (and the variable itself) is a leftover artifact from the previous inner for(;;) loop pattern that was removed in this refactor and should be cleaned up.

Suggested change

	context.awaitingAsyncContinuation = undefined
	route = '/api/tools/resume'
	payload = {
	checkpointId: continuation.checkpointId,
	}
	resumeReady = true
	if (!resumeReady) {
	break
	}
	context.awaitingAsyncContinuation = undefined
	route = '/api/tools/resume'
	payload = {
	checkpointId: continuation.checkpointId,
	}

[greptile-apps]

Unreferenced variable declaration

resumeReady is declared here but the only value it ever holds when the if (!resumeReady) guard is reached is true. The declaration can be removed along with the guard.

Suggested change

let resumeReady = false