Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion apps/sim/app/api/knowledge/[id]/documents/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
bulkKnowledgeDocumentsContract,
createKnowledgeDocumentsContract,
listKnowledgeDocumentsQuerySchema,
parseDocumentTagFiltersParam,
} from '@/lib/api/contracts/knowledge'
import { parseRequest } from '@/lib/api/server'
import { getSession } from '@/lib/auth'
Expand Down Expand Up @@ -67,6 +68,18 @@ export const GET = withRouteHandler(
const { enabledFilter, search, limit, offset, sortBy, sortOrder, tagFilters } =
queryResult.data

let parsedTagFilters: TagFilterCondition[] | undefined
try {
parsedTagFilters = parseDocumentTagFiltersParam(tagFilters) as
| TagFilterCondition[]
| undefined
} catch {
return NextResponse.json(
{ error: 'tagFilters must be a valid JSON array' },
{ status: 400 }
)
}

const result = await getDocuments(
knowledgeBaseId,
{
Expand All @@ -76,7 +89,7 @@ export const GET = withRouteHandler(
offset,
...(sortBy && { sortBy }),
...(sortOrder && { sortOrder }),
tagFilters: tagFilters as TagFilterCondition[] | undefined,
tagFilters: parsedTagFilters,
},
requestId
)
Expand Down
29 changes: 25 additions & 4 deletions apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,20 @@ export function KnowledgeBase({
const activeTagFilters: DocumentTagFilter[] = useMemo(
() =>
tagFilterEntries
.filter((f) => f.tagSlot && f.value.trim())
.filter((f) => {
if (!f.tagSlot || !f.value.trim()) return false
// A `between` filter only applies once both bounds are set. Sending it
// with just the lower bound would be rejected at the API boundary and
// break the whole list while the user is still entering the range.
if (f.operator === 'between' && !f.valueTo.trim()) return false
return true
})
.map((f) => ({
tagSlot: f.tagSlot,
fieldType: f.fieldType,
operator: f.operator,
value: f.value,
...(f.operator === 'between' && f.valueTo ? { valueTo: f.valueTo } : {}),
...(f.operator === 'between' ? { valueTo: f.valueTo } : {}),
})),
[tagFilterEntries]
)
Expand Down Expand Up @@ -1466,11 +1473,25 @@ const createEmptyEntry = (): TagFilterEntry => ({
tagName: '',
tagSlot: '',
fieldType: 'text',
operator: 'eq',
operator: 'contains',
value: '',
valueTo: '',
})

/**
* Default operator when a tag is selected. Text filters default to `contains`
* so typing part of a value finds matches (exact `equals` stays one click away
* in the operator dropdown); other field types keep their first, equality
* operator.
*/
function getDefaultOperatorForFieldType(
fieldType: FilterFieldType,
operators: ReturnType<typeof getOperatorsForFieldType>
): string {
if (fieldType === 'text') return 'contains'
return operators[0]?.value ?? 'eq'
}

interface TagFilterSectionProps {
tagDefinitions: TagDefinition[]
entries: TagFilterEntry[]
Expand Down Expand Up @@ -1601,7 +1622,7 @@ function TagFilterSection({ tagDefinitions, entries, onChange }: TagFilterSectio
tagName,
tagSlot: def?.tagSlot || '',
fieldType,
operator: operators[0]?.value || 'eq',
operator: getDefaultOperatorForFieldType(fieldType, operators),
value: '',
valueTo: '',
})
Expand Down
71 changes: 71 additions & 0 deletions apps/sim/lib/api/client/request.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* @vitest-environment node
*/
import { afterEach, describe, expect, it, vi } from 'vitest'
import { z } from 'zod'
import { requestJson } from '@/lib/api/client/request'
import { listKnowledgeDocumentsContract } from '@/lib/api/contracts/knowledge'
import { defineRouteContract } from '@/lib/api/contracts/types'

/**
* Captures the URL of the last fetch call and returns a valid JSON response so
* `requestJson`'s response validation passes.
*/
function mockFetchReturning(body: unknown) {
const fetchMock = vi.fn(
async () =>
new Response(JSON.stringify(body), {
status: 200,
headers: { 'content-type': 'application/json' },
})
)
vi.stubGlobal('fetch', fetchMock)
return fetchMock
}

describe('requestJson query serialization', () => {
afterEach(() => {
vi.unstubAllGlobals()
})

it('serializes a JSON-string query param verbatim (regression: tagFilters)', async () => {
const fetchMock = mockFetchReturning({
success: true,
data: {
documents: [],
pagination: { total: 0, limit: 50, offset: 0, hasMore: false },
},
})

const tagFilters = JSON.stringify([
{ tagSlot: 'tag1', fieldType: 'text', operator: 'contains', value: 'Ada Lovelace' },
])

await requestJson(listKnowledgeDocumentsContract, {
params: { id: 'kb-1' },
query: { tagFilters },
})

const calledUrl = String(fetchMock.mock.calls[0][0])
const url = new URL(calledUrl, 'https://example.test')
// The param must round-trip as the exact JSON, never "[object Object]".
expect(url.searchParams.get('tagFilters')).toBe(tagFilters)
expect(calledUrl).not.toContain('object+Object')
expect(calledUrl).not.toContain('[object Object]')
})

it('throws instead of silently corrupting an array-of-objects query param', async () => {
mockFetchReturning({ ok: true })

const badContract = defineRouteContract({
method: 'GET',
path: '/api/test',
query: z.object({ items: z.array(z.object({ a: z.string() })) }),
response: { mode: 'json', schema: z.object({ ok: z.boolean() }) },
})

await expect(requestJson(badContract, { query: { items: [{ a: 'x' }] } })).rejects.toThrow(
/arrays of objects are not URL-safe/
)
})
})
14 changes: 12 additions & 2 deletions apps/sim/lib/api/client/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,19 @@ function appendQuery(path: string, query: unknown): string {

if (Array.isArray(value)) {
for (const item of value) {
if (item !== undefined && item !== null && item !== '') {
searchParams.append(key, String(item))
if (item === undefined || item === null || item === '') continue
// A non-scalar in a query array would stringify to "[object Object]" and
// silently corrupt the request. Encode such values as a single JSON
// string param and decode them server-side instead. Failing loudly here
// keeps the boundary honest (this is how the knowledge tagFilters bug
// shipped undetected).
if (typeof item === 'object') {
throw new Error(
`Cannot serialize query param "${key}": arrays of objects are not URL-safe — ` +
'encode the value as a JSON string param and decode it server-side.'
)
}
searchParams.append(key, String(item))
}
continue
}
Expand Down
145 changes: 145 additions & 0 deletions apps/sim/lib/api/contracts/knowledge/documents.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import {
listKnowledgeDocumentsQuerySchema,
parseDocumentTagFiltersParam,
} from '@/lib/api/contracts/knowledge/documents'

describe('listKnowledgeDocumentsQuerySchema.tagFilters', () => {
it('keeps tagFilters a raw string (must NOT transform to an array)', () => {
// A transform-to-array here breaks requestJson outbound serialization
// (the array serializes as "[object Object]"). The wire type must stay a
// string; decoding happens server-side via parseDocumentTagFiltersParam.
const tagFilters = JSON.stringify([
{ tagSlot: 'tag1', fieldType: 'text', operator: 'contains', value: 'x' },
])
const parsed = listKnowledgeDocumentsQuerySchema.parse({ tagFilters })
expect(parsed.tagFilters).toBe(tagFilters)
expect(typeof parsed.tagFilters).toBe('string')
})
})

describe('parseDocumentTagFiltersParam', () => {
it('returns undefined for an absent param', () => {
expect(parseDocumentTagFiltersParam(undefined)).toBeUndefined()
expect(parseDocumentTagFiltersParam('')).toBeUndefined()
})

it('decodes a valid JSON array of filters', () => {
const filters = [
{ tagSlot: 'tag1', fieldType: 'text', operator: 'contains', value: 'x' },
{ tagSlot: 'date1', fieldType: 'date', operator: 'eq', value: '2026-04-21' },
]
expect(parseDocumentTagFiltersParam(JSON.stringify(filters))).toEqual(filters)
})

it('throws on malformed JSON', () => {
expect(() => parseDocumentTagFiltersParam('[object Object]')).toThrow()
expect(() => parseDocumentTagFiltersParam('{not json')).toThrow()
})

it('throws when the shape is wrong', () => {
expect(() => parseDocumentTagFiltersParam(JSON.stringify([{ tagSlot: '' }]))).toThrow()
})

it('rejects an operator that is not valid for the field type', () => {
// unknown operator
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([{ tagSlot: 'tag1', fieldType: 'text', operator: 'bogus', value: 'x' }])
)
).toThrow()
// valid operator name, wrong field type (contains is text-only)
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([
{ tagSlot: 'number1', fieldType: 'number', operator: 'contains', value: '1' },
])
)
).toThrow()
})

it('rejects a fieldType that does not match the tag slot', () => {
// number1 is a numeric column; claiming it is text must fail
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([
{ tagSlot: 'number1', fieldType: 'text', operator: 'contains', value: 'x' },
])
)
).toThrow()
})

it('rejects an unknown tag slot', () => {
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([{ tagSlot: 'tag99', fieldType: 'text', operator: 'eq', value: 'x' }])
)
).toThrow()
})

it('rejects values that are unusable for the field type', () => {
// non-numeric value on a number field
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([{ tagSlot: 'number1', fieldType: 'number', operator: 'eq', value: 'abc' }])
)
).toThrow()
// non-date value on a date field
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([{ tagSlot: 'date1', fieldType: 'date', operator: 'eq', value: 'nope' }])
)
).toThrow()
// well-formed but impossible calendar dates (would 500 on ::date)
for (const value of ['2026-02-30', '2026-99-99', '2026-13-01', '2026-00-10']) {
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([{ tagSlot: 'date1', fieldType: 'date', operator: 'eq', value }])
)
).toThrow()
}
// non-boolean value on a boolean field
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([
{ tagSlot: 'boolean1', fieldType: 'boolean', operator: 'eq', value: 'maybe' },
])
)
).toThrow()
})

it('rejects a between filter missing a usable upper bound', () => {
expect(() =>
parseDocumentTagFiltersParam(
JSON.stringify([
{
tagSlot: 'number1',
fieldType: 'number',
operator: 'between',
value: '1',
valueTo: 'x',
},
])
)
).toThrow()
})

it('accepts a valid number, date, boolean, and between filter', () => {
const filters = [
{ tagSlot: 'number1', fieldType: 'number', operator: 'gte', value: '42' },
{ tagSlot: 'date1', fieldType: 'date', operator: 'eq', value: '2026-04-21' },
{ tagSlot: 'boolean1', fieldType: 'boolean', operator: 'eq', value: 'true' },
{
tagSlot: 'number2',
fieldType: 'number',
operator: 'between',
value: '1',
valueTo: '10',
},
]
expect(parseDocumentTagFiltersParam(JSON.stringify(filters))).toEqual(filters)
})
})
Loading
Loading