Skip to content

Commit 8db2bd6

Browse files
committed
fix: close the gaps an adversarial review of this branch found
A conflict handler added earlier in this branch was dead code. It read the Postgres error code off the thrown object, but the driver error arrives wrapped with the real one on `cause`, so the check returned false on its first line and the 409 never fired. Its test passed only because it threw a flat shape production never produces. It now reads through the cause chain with the shared helpers, compares the constraint name exactly instead of matching a substring of the SQL, and its test throws the real wrapped error. Resuming a conversation checked its workflow and its workspace but not its type, so a conversation created by the web surface could be continued as a CLI turn. It now refuses through the same uniform 404 as every other mismatch, which closes the same omission on the web posting path. Minting one no longer leaves a blank untitled row at the top of the Chat list. The pre-write check on a minted API key refused fewer characters than the writer does, so a key the check accepted could still fail at the write — after the endpoint beside it was already stored, pairing a new endpoint with the previous key. The two had drifted because the set was spelled three times; there is now one. A description claimed a processed count reported only the chunks that changed. The update returns every row it matched, so re-enabling chunks that were already enabled counts them all. Two OpenAPI sentences promised no conflict detection and no persistence warnings in a dry run, both of which the same branch had just made false. A described window was wrong whenever a start was supplied without an end. Listing the editors of a built-in skill answered a read with a modification refusal on the internal surface. Archived table listings could reach the strict folder projector again through a third scope value the input type still allowed. A metadata-only secret write skipped the guard its personal-scope twin has. The internal document boundary still took the two processing fields as unbounded strings. Truncation was reported only from the response envelope, so a clipped file body, row search and workflow-stats list said nothing. A staged download stopped watching for signals before it finished removing its directory, and cleared every listener for the signal rather than its own. Three tests asserted a contract constant against itself; they now drive rendered help, real argv, or real render output.
1 parent 6a4b33b commit 8db2bd6

40 files changed

Lines changed: 887 additions & 195 deletions

apps/docs/content/docs/en/cli/scripting.mdx

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ printf 'wf_3Qm8ZtLpR2yVnKd7BsXwC\nwf_5Hn1JvTqW9xUcMb4RzPgL\n' | sim files mv --f
3131

3232
Arrays of objects stay JSON.
3333

34+
## Passing a literal leading `@`
35+
36+
Because `@` introduces a file reference, a value that genuinely starts with one
37+
is written `@@`. Only the leading `@` is dropped, and every `@`-aware flag
38+
accepts the escape:
39+
40+
```bash
41+
sim files share set wf_3Qm8ZtLpR2yVnKd7BsXwC --allowed-emails @@example.org
42+
sim secrets set API_HOST --value @@internal
43+
```
44+
45+
Without it, `--allowed-emails @example.org` can only be read as a request to
46+
open a file named `example.org`.
47+
3448
## Filtering table rows
3549

3650
`--filter` takes the same predicate tree the API uses: `all` (AND) or `any` (OR)
@@ -117,11 +131,23 @@ esac
117131

118132
## Selecting workflow output
119133

120-
`--select-output` takes `blockName.field` selectors. Fields that a run did not
121-
produce are simply omitted:
134+
`--select-output` shapes a streamed result, so it requires `--follow`. It takes
135+
`blockName.field` selectors; fields that a run did not produce are simply
136+
omitted:
137+
138+
```bash
139+
sim workflows run 3a9e21d8-5f47-4c0b-b2ea-91d7c6034ef8 --follow --select-output agent_1.content --output json
140+
```
141+
142+
Without `--follow` the CLI refuses the pair rather than spending a request on a
143+
response that carries no outputs, and `--async` cannot be combined with it
144+
either — there is no stream to shape. To narrow a run that has already finished,
145+
read it back with `workflows runs get`, which matches block **ids** rather than
146+
the block names `workflows run` takes:
122147

123148
```bash
124-
sim workflows run 3a9e21d8-5f47-4c0b-b2ea-91d7c6034ef8 --select-output agent_1.content --output json
149+
sim workflows runs get "$run_id" --workflow 3a9e21d8-5f47-4c0b-b2ea-91d7c6034ef8 \
150+
--select-output 1d4c8f02-7b63-4a19-8e52-63f0a7c5d9b1.content --output json
125151
```
126152

127153
## Polling a long run

apps/sim/app/api/mothership/chats/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { parseRequest } from '@/lib/api/server'
1010
import { listMothershipChats } from '@/lib/copilot/chat/list-mothership-chats'
1111
import { chatPubSub } from '@/lib/copilot/chat-status'
12+
import { MOTHERSHIP_CHAT_DEFAULT_MODEL } from '@/lib/copilot/constants'
1213
import {
1314
authenticateCopilotRequestSessionOnly,
1415
createForbiddenResponse,
@@ -78,7 +79,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7879
workspaceId,
7980
type: 'mothership',
8081
title: null,
81-
model: 'claude-opus-4-8',
82+
model: MOTHERSHIP_CHAT_DEFAULT_MODEL,
8283
updatedAt: now,
8384
lastSeenAt: now,
8485
})

apps/sim/app/api/v2/chat/route.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,42 @@ describe('POST /api/v2/chat', () => {
325325
expect(mockRunHeadlessCopilotLifecycle).not.toHaveBeenCalled()
326326
})
327327

328+
it('asks the resolver for a mothership conversation, so another type resolves to nothing', async () => {
329+
await callChat({
330+
workspaceId: 'workspace-1',
331+
message: 'and then?',
332+
conversationId: OWNED_CONVERSATION_ID,
333+
})
334+
335+
expect(mockResolveOrCreateChat).toHaveBeenCalledWith(
336+
expect.objectContaining({ chatId: OWNED_CONVERSATION_ID, type: 'mothership' })
337+
)
338+
})
339+
340+
it('titles a new conversation by its first message so the web Chat list has no blank row', async () => {
341+
await callChat({ workspaceId: 'workspace-1', message: ' Summarize\n last week ' })
342+
343+
expect(mockResolveOrCreateChat).toHaveBeenCalledWith(
344+
expect.objectContaining({ title: 'Summarize last week' })
345+
)
346+
})
347+
348+
it('truncates a long first message into a title instead of storing the whole message', async () => {
349+
const message = 'a'.repeat(500)
350+
351+
await callChat({ workspaceId: 'workspace-1', message })
352+
353+
const title = (mockResolveOrCreateChat.mock.calls[0][0] as { title?: string }).title
354+
expect(title).toBe(`${'a'.repeat(80)}...`)
355+
})
356+
357+
it('leaves the title unset for a whitespace-only message rather than stamping an empty one', async () => {
358+
await callChat({ workspaceId: 'workspace-1', message: ' \n ' })
359+
360+
const resolverInput = mockResolveOrCreateChat.mock.calls[0][0] as Record<string, unknown>
361+
expect(Object.hasOwn(resolverInput, 'title')).toBe(false)
362+
})
363+
328364
it('rejects a malformed conversation id before resolving anything', async () => {
329365
const response = await callChat({
330366
workspaceId: 'workspace-1',

apps/sim/app/api/v2/chat/route.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { generateId } from '@sim/utils/id'
4+
import { truncate } from '@sim/utils/string'
45
import type { NextRequest } from 'next/server'
56
import { v2ChatContract } from '@/lib/api/contracts/v2/chat'
67
import { parseRequest } from '@/lib/api/server'
@@ -16,6 +17,7 @@ import { chatOperations } from '@/lib/copilot/application/operations'
1617
import { resolveOrCreateChat } from '@/lib/copilot/chat/lifecycle'
1718
import { buildIntegrationToolSchemas } from '@/lib/copilot/chat/payload'
1819
import { generateWorkspaceContext } from '@/lib/copilot/chat/workspace-context'
20+
import { MOTHERSHIP_CHAT_DEFAULT_MODEL } from '@/lib/copilot/constants'
1921
import { computeWorkspaceEntitlements } from '@/lib/copilot/entitlements'
2022
import {
2123
type CopilotEnvironmentContext,
@@ -50,10 +52,23 @@ const CHAT_HEARTBEAT_INTERVAL_MS = 15_000
5052
const ndjsonEncoder = new TextEncoder()
5153

5254
/**
53-
* Model recorded on conversations this route creates, matching the model the
54-
* web Chat surface stamps on a new mothership conversation.
55+
* Longest title derived from a first message. Well under the 200-character
56+
* ceiling the rename contract enforces, and short enough to read as one line
57+
* in the web Chat list.
5558
*/
56-
const V2_CHAT_MODEL = 'claude-opus-4-8'
59+
const CHAT_TITLE_MAX_LENGTH = 80
60+
61+
/**
62+
* Title a conversation this route creates by its first message, so a `sim chat`
63+
* turn does not leave a blank row at the top of the user's web Chat list.
64+
* Returns undefined for a message that is only whitespace, leaving the title
65+
* unset rather than stamping an empty one.
66+
*/
67+
function deriveConversationTitle(message: string): string | undefined {
68+
const normalized = message.replace(/\s+/g, ' ').trim()
69+
if (!normalized) return undefined
70+
return truncate(normalized, CHAT_TITLE_MAX_LENGTH)
71+
}
5772

5873
function isAbortError(error: unknown): boolean {
5974
return error instanceof Error && error.name === 'AbortError'
@@ -145,6 +160,8 @@ export const POST = withRouteHandler(
145160
const workspaceAccess = await assertActiveWorkspaceAccess(workspaceId, userId)
146161
const userPermission = workspaceAccess.permission
147162

163+
const conversationTitle = deriveConversationTitle(message)
164+
148165
// A caller-supplied conversation id is a claim, not an identity: resolve
149166
// it through the same owner- and workspace-scoped loader the web Chat
150167
// surface uses, and refuse every id that does not resolve with the same
@@ -154,8 +171,9 @@ export const POST = withRouteHandler(
154171
...(conversationId ? { chatId: conversationId } : {}),
155172
userId,
156173
workspaceId,
157-
model: V2_CHAT_MODEL,
174+
model: MOTHERSHIP_CHAT_DEFAULT_MODEL,
158175
type: 'mothership',
176+
...(conversationTitle ? { title: conversationTitle } : {}),
159177
})
160178
if (conversationId && !resolvedChat.chat) {
161179
return v2Error('NOT_FOUND', 'Conversation not found')

apps/sim/app/api/v2/workspaces/[workspaceId]/route.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ describe.each(routes)('v2 $name workspace concealment', ({ spy, call }) => {
121121
const response = await call()
122122

123123
expect(response.status).toBe(403)
124-
expect(await response.json()).toMatchObject({ error: { code: 'FORBIDDEN' } })
124+
expect(await response.json()).toMatchObject({
125+
error: { code: 'FORBIDDEN', details: { code: 'INSUFFICIENT_WORKSPACE_ROLE' } },
126+
})
125127
})
126128
})

apps/sim/lib/api/contracts/knowledge/documents.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
*/
44
import { describe, expect, it } from 'vitest'
55
import {
6+
bulkCreateDocumentsBodySchema,
67
listKnowledgeDocumentsQuerySchema,
78
parseDocumentTagFiltersParam,
9+
upsertDocumentBodySchema,
810
} from '@/lib/api/contracts/knowledge/documents'
911

1012
describe('listKnowledgeDocumentsQuerySchema.tagFilters', () => {
@@ -143,3 +145,53 @@ describe('parseDocumentTagFiltersParam', () => {
143145
expect(parseDocumentTagFiltersParam(JSON.stringify(filters))).toEqual(filters)
144146
})
145147
})
148+
149+
/**
150+
* `recipe` and `lang` reach analytics and nothing else, so an unrecognised value
151+
* was accepted with a 200 and silently discarded. Both internal write bodies
152+
* reuse the upload boundary's validated shape, which is what every first-party
153+
* caller already sends.
154+
*/
155+
describe('internal document processingOptions', () => {
156+
const DOCUMENT = {
157+
filename: 'notes.txt',
158+
fileUrl: 'https://example.com/notes.txt',
159+
fileSize: 12,
160+
mimeType: 'text/plain',
161+
}
162+
163+
const boundaries = [
164+
{
165+
name: 'bulk create',
166+
parse: (processingOptions: unknown) =>
167+
bulkCreateDocumentsBodySchema.safeParse({
168+
documents: [DOCUMENT],
169+
bulk: true,
170+
processingOptions,
171+
}),
172+
},
173+
{
174+
name: 'upsert',
175+
parse: (processingOptions: unknown) =>
176+
upsertDocumentBodySchema.safeParse({ ...DOCUMENT, processingOptions }),
177+
},
178+
] as const
179+
180+
describe.each(boundaries)('$name', ({ parse }) => {
181+
it('accepts what the shipped first-party callers send', () => {
182+
expect(parse({ recipe: 'default', lang: 'en' }).success).toBe(true)
183+
})
184+
185+
it('rejects an unrecognised recipe instead of discarding it', () => {
186+
const result = parse({ recipe: 'super-chunker-9000', lang: 'en' })
187+
expect(result.success).toBe(false)
188+
expect(result.error?.issues[0]?.path).toEqual(['processingOptions', 'recipe'])
189+
})
190+
191+
it('rejects a lang that is not a BCP-47 tag', () => {
192+
const result = parse({ recipe: 'default', lang: 'en_US' })
193+
expect(result.success).toBe(false)
194+
expect(result.error?.issues[0]?.path).toEqual(['processingOptions', 'lang'])
195+
})
196+
})
197+
})

apps/sim/lib/api/contracts/knowledge/documents.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types'
1717
import { PRIVATE_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata'
1818
import { getFieldTypeForSlot, MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE } from '@/lib/knowledge/constants'
1919
import { getOperatorsForFieldType, isValidFilterValue } from '@/lib/knowledge/filters/types'
20+
import { knowledgeDocumentUploadMetadataSchema } from '@/lib/knowledge/upload-metadata'
2021

2122
export const documentTagFilterSchema = z
2223
.object({
@@ -136,12 +137,7 @@ export const bulkCreateDocumentsBodySchema = z.object({
136137
MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE,
137138
`At most ${MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE} documents may be created at once`
138139
),
139-
processingOptions: z
140-
.object({
141-
recipe: z.string().optional(),
142-
lang: z.string().optional(),
143-
})
144-
.optional(),
140+
processingOptions: knowledgeDocumentUploadMetadataSchema.shape.processingOptions,
145141
bulk: z.literal(true),
146142
workflowId: z.string().optional(),
147143
[PRIVATE_SECRET_PROVENANCE_FIELD]: privateSecretProvenanceBundleSchema.optional(),
@@ -173,12 +169,7 @@ export const upsertDocumentBodySchema = z.object({
173169
fileSize: z.number().min(1, 'File size must be greater than 0'),
174170
mimeType: z.string().min(1, 'MIME type is required'),
175171
documentTagsData: z.string().optional(),
176-
processingOptions: z
177-
.object({
178-
recipe: z.string().optional(),
179-
lang: z.string().optional(),
180-
})
181-
.optional(),
172+
processingOptions: knowledgeDocumentUploadMetadataSchema.shape.processingOptions,
182173
workflowId: z.string().optional(),
183174
[PRIVATE_SECRET_PROVENANCE_FIELD]: privateSecretProvenanceBundleSchema.optional(),
184175
})

0 commit comments

Comments
 (0)