Skip to content

Commit a37e9ea

Browse files
committed
revert(mothership): keep the confirm trace outcome vocabulary unchanged
Drop the held_by_desktop outcome so this change needs no trace contract update: the 409 paths record tool_call_not_found as before. Keep a route test for a not-started report on a claimed call.
1 parent b9c1897 commit a37e9ea

3 files changed

Lines changed: 8 additions & 37 deletions

File tree

‎apps/sim/app/api/copilot/confirm/route.test.ts‎

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
import { trace } from '@opentelemetry/api'
2-
import {
3-
BasicTracerProvider,
4-
InMemorySpanExporter,
5-
SimpleSpanProcessor,
6-
} from '@opentelemetry/sdk-trace-base'
71
import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing'
82
import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock'
93
import {
@@ -12,7 +6,7 @@ import {
126
} from '@sim/testing/mocks/mothership-async-runs.mock'
137
import { createMockRequest } from '@sim/testing/mocks/request.mock'
148
import type { NextRequest } from 'next/server'
15-
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
import { beforeEach, describe, expect, it, vi } from 'vitest'
1610

1711
const { publishToolConfirmation, getTrustedWorkflowToolExecution } = vi.hoisted(() => ({
1812
publishToolConfirmation: vi.fn(),
@@ -33,9 +27,6 @@ vi.mock('@/lib/workflows/executor/execution-state', () => ({
3327
getTrustedWorkflowToolExecution,
3428
}))
3529

36-
import { CopilotConfirmOutcome } from '@/lib/mothership/generated/trace-attribute-values-v1'
37-
import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1'
38-
import { TraceSpan } from '@/lib/mothership/generated/trace-spans-v1'
3930
import { POST } from './route'
4031

4132
const {
@@ -49,17 +40,6 @@ const {
4940

5041
const encryptSecret = encryptionMockFns.mockEncryptSecret
5142

52-
/** Records the confirm spans and returns a reader for the outcome the route recorded. */
53-
function recordConfirmOutcome(): () => unknown {
54-
const exporter = new InMemorySpanExporter()
55-
trace.setGlobalTracerProvider(
56-
new BasicTracerProvider({ spanProcessors: [new SimpleSpanProcessor(exporter)] })
57-
)
58-
return () =>
59-
exporter.getFinishedSpans().find((span) => span.name === TraceSpan.CopilotConfirmToolResult)
60-
?.attributes[TraceAttr.CopilotConfirmOutcome]
61-
}
62-
6343
describe('Copilot Confirm API Route', () => {
6444
const existingRow = {
6545
toolCallId: 'tool-call-123',
@@ -71,10 +51,6 @@ describe('Copilot Confirm API Route', () => {
7151
claimedBy: 'workflow:execution-1',
7252
}
7353

74-
afterEach(() => {
75-
trace.disable()
76-
})
77-
7854
beforeEach(() => {
7955
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValue({
8056
userId: 'user-1',
@@ -175,7 +151,6 @@ describe('Copilot Confirm API Route', () => {
175151
})
176152

177153
it('rejects a native success before the desktop authorization claim', async () => {
178-
const recordedOutcome = recordConfirmOutcome()
179154
getAsyncToolCall.mockResolvedValue({
180155
...existingRow,
181156
toolName: 'browser_snapshot',
@@ -191,7 +166,6 @@ describe('Copilot Confirm API Route', () => {
191166
)
192167

193168
expect(response.status).toBe(409)
194-
expect(recordedOutcome()).toBe(CopilotConfirmOutcome.HeldByDesktop)
195169
expect(completeAsyncToolCall).not.toHaveBeenCalled()
196170
expect(detachAsyncToolCall).not.toHaveBeenCalled()
197171
expect(encryptSecret).not.toHaveBeenCalled()
@@ -244,7 +218,6 @@ describe('Copilot Confirm API Route', () => {
244218
] as const)(
245219
'rejects a pending %s %s when the native authorization claim wins the race',
246220
async (toolName, status) => {
247-
const recordedOutcome = recordConfirmOutcome()
248221
getAsyncToolCall.mockResolvedValue({
249222
...existingRow,
250223
toolName,
@@ -264,7 +237,6 @@ describe('Copilot Confirm API Route', () => {
264237
expect(await response.json()).toEqual({
265238
error: 'The desktop app holds this tool call; only its own result settles it',
266239
})
267-
expect(recordedOutcome()).toBe(CopilotConfirmOutcome.HeldByDesktop)
268240
expect(completePendingAsyncToolCall).toHaveBeenCalledOnce()
269241
expect(completeClaimedAsyncToolCall).not.toHaveBeenCalled()
270242
expect(completeAsyncToolCall).not.toHaveBeenCalled()
@@ -314,7 +286,6 @@ describe('Copilot Confirm API Route', () => {
314286
)
315287

316288
it('refuses a not-started report for a call the desktop already claimed', async () => {
317-
const recordedOutcome = recordConfirmOutcome()
318289
getAsyncToolCall.mockResolvedValue({
319290
...existingRow,
320291
toolName: 'browser_snapshot',
@@ -335,7 +306,6 @@ describe('Copilot Confirm API Route', () => {
335306
expect(await response.json()).toEqual({
336307
error: 'The desktop app holds this tool call; only its own result settles it',
337308
})
338-
expect(recordedOutcome()).toBe(CopilotConfirmOutcome.HeldByDesktop)
339309
})
340310

341311
it('does not publish when another terminal transition wins indeterminate claim reconciliation', async () => {

‎apps/sim/app/api/copilot/confirm/route.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ function acknowledgeSettledToolCall(
8989
* report raced that claim and lost), so only the claim's own result settles it. Final, not
9090
* retryable: the reporter stops.
9191
*/
92-
function heldByAnotherReporterResponse(span: Span): NextResponse {
93-
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.HeldByDesktop)
92+
function heldByAnotherReporterResponse(): NextResponse {
9493
return NextResponse.json(
9594
{ error: 'The desktop app holds this tool call; only its own result settles it' },
9695
{ status: 409 }
@@ -286,7 +285,8 @@ export const POST = withRouteHandler((req: NextRequest) => {
286285
? isWorkflowToolExecutionClaimable(existing.status, existing.permissionDecision)
287286
: existing.status === ASYNC_TOOL_STATUS.running || isPreclaimNativeTerminalOutcome
288287
if (isNativeClientTool && !isMutableClientToolCall) {
289-
return heldByAnotherReporterResponse(span)
288+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
289+
return heldByAnotherReporterResponse()
290290
}
291291
if (isWorkflowTool && !isMutableClientToolCall) {
292292
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
@@ -300,7 +300,8 @@ export const POST = withRouteHandler((req: NextRequest) => {
300300
data.notStarted === true &&
301301
existing.status !== ASYNC_TOOL_STATUS.pending
302302
) {
303-
return heldByAnotherReporterResponse(span)
303+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
304+
return heldByAnotherReporterResponse()
304305
}
305306

306307
let effectiveStatus = status
@@ -436,7 +437,8 @@ export const POST = withRouteHandler((req: NextRequest) => {
436437
}
437438

438439
if (reconciledOutcome === 'conflict' && isPreclaimNativeTerminalOutcome) {
439-
return heldByAnotherReporterResponse(span)
440+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
441+
return heldByAnotherReporterResponse()
440442
}
441443

442444
if (reconciledOutcome !== 'updated') {

‎apps/sim/lib/mothership/generated/trace-attribute-values-v1.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ export type CopilotChatPersistOutcomeValue =
115115
export const CopilotConfirmOutcome = {
116116
Delivered: 'delivered',
117117
Forbidden: 'forbidden',
118-
HeldByDesktop: 'held_by_desktop',
119118
InternalError: 'internal_error',
120119
RunNotFound: 'run_not_found',
121120
ToolCallNotFound: 'tool_call_not_found',

0 commit comments

Comments
 (0)