Skip to content

Commit c0bf037

Browse files
Sg312icecrasher321
authored andcommitted
fix(plan): address eligibility, benchmark state and desktop review findings
1 parent 1522f2f commit c0bf037

83 files changed

Lines changed: 154638 additions & 2645 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/desktop/src/main/computer-use/native-client.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ describe('native computer use transport', () => {
6262
expect(results).toEqual([JSON.parse(status), JSON.parse(status)])
6363
})
6464

65+
it('accepts a maximum-sized frame followed by another valid frame', async () => {
66+
const { client, reset } = helper(`let first; ${reader}
67+
if (!first) { first = request; return; }
68+
const a = JSON.stringify({id:first.id,result:${status}});
69+
const b = JSON.stringify({id:request.id,result:${status}});
70+
process.stdout.write(a.padEnd(16 * 1024 * 1024, ' ') + '\\n' + b + '\\n');
71+
});`)
72+
const results = await Promise.all([client.request('status', {}), client.request('status', {})])
73+
expect(results).toEqual([JSON.parse(status), JSON.parse(status)])
74+
expect(reset).not.toHaveBeenCalled()
75+
})
76+
6577
it('rejects malformed output and can start a fresh helper afterward', async () => {
6678
const { client, reset } = helper(`${reader}
6779
process.stdout.write(request.method==='bad'?'not json\\n':JSON.stringify({id:request.id,result:${status}})+'\\n');

‎apps/desktop/src/main/computer-use/native-client.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ export class NativeComputerUseClient implements ComputerUseNativeClient {
6666
child.stdout.on('data', (chunk: Buffer) => {
6767
if (this.child !== child) return
6868
this.buffer = Buffer.concat([this.buffer, chunk])
69-
if (this.buffer.length > MAX_FRAME_BYTES) {
70-
this.stopWithError(new Error('Computer Use returned an oversized response.'))
71-
return
72-
}
7369
let newline = this.buffer.indexOf(10)
7470
while (newline >= 0) {
71+
if (newline > MAX_FRAME_BYTES) {
72+
this.stopWithError(new Error('Computer Use returned an oversized response.'))
73+
return
74+
}
7575
const line = this.buffer.subarray(0, newline).toString('utf8')
7676
this.buffer = this.buffer.subarray(newline + 1)
7777
try {
@@ -89,6 +89,8 @@ export class NativeComputerUseClient implements ComputerUseNativeClient {
8989
}
9090
newline = this.buffer.indexOf(10)
9191
}
92+
if (this.buffer.length > MAX_FRAME_BYTES)
93+
this.stopWithError(new Error('Computer Use returned an oversized response.'))
9294
})
9395
/** Native diagnostics must never copy app contents into application logs. */
9496
child.stderr.resume()

‎apps/desktop/src/main/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ import {
88
BrowserWindow,
99
crashReporter,
1010
dialog,
11+
globalShortcut,
1112
Notification,
1213
net,
1314
powerSaveBlocker,
14-
globalShortcut,
1515
session,
1616
shell,
1717
} from 'electron'

‎apps/desktop/src/main/ipc.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import {
99
isCurrentBrowserToolName,
1010
} from '@sim/browser-protocol'
1111
import {
12-
type DesktopExecutorDevice,
1312
ComputerUseError,
1413
type ComputerUseToolFailure,
14+
type DesktopExecutorDevice,
1515
type DesktopNotificationPayload,
1616
type DesktopServerChangeResult,
1717
type DesktopServerConfiguration,

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

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -349,28 +349,36 @@ describe('Copilot Confirm API Route', () => {
349349
}
350350
)
351351

352-
it('completes a computer result only through its exact native claim', async () => {
353-
getAsyncToolCall.mockResolvedValue({
354-
...existingRow,
355-
toolName: 'computer',
356-
claimedBy: 'desktop-computer',
357-
})
358-
const response = await POST(
359-
createMockPostRequest({ toolCallId: 'tool-call-123', status: 'success', data: { ok: true } })
360-
)
361-
expect(response.status).toBe(200)
362-
expect(completeClaimedAsyncToolCall).toHaveBeenCalledWith(
363-
{
364-
toolCallId: 'tool-call-123',
365-
status: 'completed',
366-
result: { __sealedClientToolCompletionV1: 'sealed-client-result' },
367-
error: null,
368-
},
369-
'desktop-computer'
370-
)
371-
expect(completeAsyncToolCall).not.toHaveBeenCalled()
372-
expect(publishToolConfirmation).toHaveBeenCalledOnce()
373-
})
352+
it.each([null, 'device-1'])(
353+
'completes a native computer claim with background device %s',
354+
async (desktopDeviceId) => {
355+
getRunSegment.mockResolvedValue({ id: 'run-1', userId: 'user-1', desktopDeviceId })
356+
getAsyncToolCall.mockResolvedValue({
357+
...existingRow,
358+
toolName: 'computer',
359+
claimedBy: 'desktop-computer',
360+
})
361+
const response = await POST(
362+
createMockPostRequest({
363+
toolCallId: 'tool-call-123',
364+
status: 'success',
365+
data: { ok: true },
366+
})
367+
)
368+
expect(response.status).toBe(200)
369+
expect(completeClaimedAsyncToolCall).toHaveBeenCalledWith(
370+
{
371+
toolCallId: 'tool-call-123',
372+
status: 'completed',
373+
result: { __sealedClientToolCompletionV1: 'sealed-client-result' },
374+
error: null,
375+
},
376+
'desktop-computer'
377+
)
378+
expect(completeAsyncToolCall).not.toHaveBeenCalled()
379+
expect(publishToolConfirmation).toHaveBeenCalledOnce()
380+
}
381+
)
374382

375383
it('rejects background computer results without detaching the native claim', async () => {
376384
getAsyncToolCall.mockResolvedValue({

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
type AsyncCompletionData,
1313
type AsyncConfirmationStatus,
1414
type AsyncTerminalStatus,
15+
DESKTOP_TOOL_CLAIM_OWNER,
1516
getTerminalConfirmationStatus,
1617
isDeliveredAsyncStatus,
1718
isTerminalAsyncStatus,
@@ -42,7 +43,7 @@ import {
4243
import { isWorkflowToolName } from '@/lib/mothership/tools/client-executed-tools'
4344
import {
4445
getDesktopToolClaimOwner,
45-
isDesktopToolCall,
46+
isBackgroundDesktopToolCall,
4647
isNativeDesktopTool,
4748
} from '@/lib/mothership/tools/desktop-tools'
4849
import {
@@ -210,7 +211,10 @@ export const POST = withRouteHandler((req: NextRequest) => {
210211
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
211212
}
212213

213-
if (run.desktopDeviceId && isDesktopToolCall(existing.toolName, toRecord(existing.args))) {
214+
if (
215+
run.desktopDeviceId &&
216+
isBackgroundDesktopToolCall(existing.toolName, toRecord(existing.args))
217+
) {
214218
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.Forbidden)
215219
return NextResponse.json(
216220
{
@@ -442,7 +446,12 @@ export const POST = withRouteHandler((req: NextRequest) => {
442446
...(isPreclaimNativeTerminalOutcome
443447
? { guard: { kind: 'pending' } as const }
444448
: existing.toolName === 'computer'
445-
? { guard: { kind: 'claimed', claimedBy: DESKTOP_TOOL_CLAIM_OWNER.computer } as const }
449+
? {
450+
guard: {
451+
kind: 'claimed',
452+
claimedBy: DESKTOP_TOOL_CLAIM_OWNER.computer,
453+
} as const,
454+
}
446455
: {}),
447456
}
448457
)

‎apps/sim/app/api/organizations/[id]/benchmarks/[benchmarkId]/route.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ export const GET = defineInternalJsonRoute({
1717
contract: getBenchmarkContract,
1818
auth: internalSessionAuth,
1919
operation: benchmarkOperations.read,
20-
rateLimit: internalRateLimits.none({
21-
reason: 'The owner polls bounded benchmark artifacts under current source access',
22-
}),
20+
rateLimit: internalRateLimits.user({ bucketName: 'benchmark-read' }),
2321
errorPolicy: internalOrchestrationErrorPolicy,
2422
beforeParse: async ({ principal }) => {
2523
await requireBenchmarkOperator(principal)

‎apps/sim/app/o/[organizationId]/benchmark/components/benchmark-detail.tsx‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ interface BenchmarkEditorProps {
4040
busy: boolean
4141
saving: boolean
4242
stage: RunBenchmarkStageBody['stage'] | null
43-
onUpdate: (body: UpdateBenchmarkBody) => void
43+
onUpdate: (body: UpdateBenchmarkBody, onSaved: () => void) => void
4444
onRun: (stage: RunBenchmarkStageBody['stage'], runLabel?: string) => void
4545
}
4646

@@ -53,7 +53,16 @@ function BenchmarkEditor({
5353
onUpdate,
5454
onRun,
5555
}: BenchmarkEditorProps) {
56-
const [draft, setDraft] = useState(benchmark.artifacts)
56+
const [edit, setEdit] = useState<{
57+
version: number
58+
artifacts: BenchmarkCase['artifacts']
59+
} | null>(null)
60+
const draft = edit?.artifacts ?? benchmark.artifacts
61+
const changeDraft = (patch: Partial<BenchmarkCase['artifacts']>) =>
62+
setEdit((current) => ({
63+
version: current?.version ?? benchmark.version,
64+
artifacts: { ...(current?.artifacts ?? benchmark.artifacts), ...patch },
65+
}))
5766
const [runLabel, setRunLabel] = useState('')
5867
const { artifacts } = benchmark
5968
const referenceDirty =
@@ -76,31 +85,32 @@ function BenchmarkEditor({
7685

7786
return (
7887
<div className='flex flex-col gap-8 divide-y divide-[var(--border)] [&>section+section]:pt-8'>
88+
{edit && edit.version !== benchmark.version && (
89+
<div role='alert' className='flex items-center gap-3 text-[var(--text-muted)] text-small'>
90+
This benchmark changed on the server. Your edits are preserved; copy any text you need
91+
before loading the latest version.
92+
<Chip onClick={() => setEdit(null)}>Discard local edits</Chip>
93+
</div>
94+
)}
7995
<BenchmarkReference
8096
artifacts={draft}
8197
busy={busy}
8298
saving={saving}
8399
stage={stage}
84100
referenceDirty={referenceDirty}
85101
redactionDirty={redactionDirty}
86-
onChange={(patch) => setDraft((current) => ({ ...current, ...patch }))}
102+
onChange={changeDraft}
87103
onSave={() => {
88104
onUpdate(
89-
referenceDirty
90-
? {
91-
version: benchmark.version,
92-
...(draft.taskBrief !== artifacts.taskBrief
93-
? { taskBrief: draft.taskBrief }
94-
: {}),
95-
...(draft.referenceSpec !== artifacts.referenceSpec
96-
? { referenceSpec: draft.referenceSpec }
97-
: {}),
98-
}
99-
: {
100-
version: benchmark.version,
101-
redactedSpec: draft.redactedSpec,
102-
blanks: draft.blanks,
103-
}
105+
{
106+
version: edit?.version ?? benchmark.version,
107+
...(draft.taskBrief !== artifacts.taskBrief ? { taskBrief: draft.taskBrief } : {}),
108+
...(draft.referenceSpec !== artifacts.referenceSpec
109+
? { referenceSpec: draft.referenceSpec }
110+
: {}),
111+
...(redactionDirty ? { redactedSpec: draft.redactedSpec, blanks: draft.blanks } : {}),
112+
},
113+
() => setEdit((current) => (current === edit ? null : current))
104114
)
105115
}}
106116
onRun={onRun}
@@ -122,8 +132,8 @@ function BenchmarkEditor({
122132
>
123133
{!canPlan && (
124134
<p className='text-[var(--text-muted)] text-small'>
125-
Plan mode requires permission to create organization workspaces. An organization
126-
administrator can update the selected user’s access.
135+
The selected user needs Plan mode access and permission to create organization
136+
workspaces.
127137
</p>
128138
)}
129139
{artifacts.generatedSpec ? (
@@ -310,15 +320,15 @@ export function BenchmarkDetail({
310320
<BenchmarkHistory organizationId={organizationId} benchmarkId={benchmarkId} />
311321
) : (
312322
<BenchmarkEditor
313-
key={`${benchmark.id}:${benchmark.version}`}
323+
key={benchmark.id}
314324
benchmark={benchmark}
315325
canPlan={canPlan}
316326
busy={busy}
317327
saving={updateBenchmark.isPending}
318328
stage={stage}
319-
onUpdate={(body) => {
329+
onUpdate={(body, onSaved) => {
320330
runStage.reset()
321-
updateBenchmark.mutate(body)
331+
updateBenchmark.mutate(body, { onSuccess: onSaved })
322332
}}
323333
onRun={(nextStage, runLabel) => {
324334
updateBenchmark.reset()

‎apps/sim/app/o/[organizationId]/benchmark/components/benchmark-history.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,10 @@ export function BenchmarkHistory({ organizationId, benchmarkId }: BenchmarkHisto
133133
Loading saved results…
134134
</p>
135135
)}
136-
{selected.data && (!baselineId || baseline.data) && (
136+
{selected.data && (
137137
<BenchmarkRunComparison
138138
run={selected.data.run}
139-
baseline={baseline.data?.run}
139+
baseline={baseline.error ? undefined : baseline.data?.run}
140140
organizationId={organizationId}
141141
/>
142142
)}

0 commit comments

Comments
 (0)