Skip to content

Commit 29b2d8b

Browse files
committed
fix: act on a second review round, and correct two earlier claims
The conflict pre-check read block ids from the wrong side. The writer inserts each block's own `id` field while the check read the record key, and the two can diverge because preparation copies a value under its key without reconciling them. Edges already read the value and subflows are genuinely keyed by the record key, so only blocks were wrong — collecting every family from the values, as first suggested, would have broken subflows instead. A minted API key carrying leading or trailing whitespace passed the pre-write check but failed the writer, leaving the new endpoint on disk beside the previous key. It is refused up front now rather than trimmed: a key is opaque, so trimming would store a value the server never issued and turn a loud failure into an unexplained 401 later. The endpoint normalizer does trim, which is what made a padded `--endpoint` fail only after the browser flow had already minted a key. A metadata-only secret write raced with deletion returned 500, because the follow-up read that only assembles the response body threw an unclassified error; it now reports the same not-found the non-racing miss already gave. An unusable output format in the environment silently printed a table instead of refusing. Two validation messages printed control characters verbatim. A dry run now reports the preparation warnings its own commit path returns. The chat route created a titled conversation and never wrote a message, so it appeared in the Chat list promising content it did not have. Both sides of a successful turn are now persisted; a failed turn still writes nothing, so a question is never stored without its answer. Two claims of mine were wrong. The earlier commit message said `secrets set` sent an empty value that overwrote the stored secret — it did not; the prompt refuses off a TTY and rejects empty on one, so the old behaviour was a clean refusal. And the delay helper commit said this package's idiom is `node:timers/promises`; the package carries its own `sleep`, which is the audit's sanctioned home and has five callers. It uses that now. Also: a test asserting a deadlock stays unclassified could not fail, since every candidate rejects it; it now pins a unique violation carrying no constraint name. Workflow ids spelled with the file prefix are corrected in the remaining fixtures, leaving the genuine file ids alone.
1 parent cb40f3c commit 29b2d8b

40 files changed

Lines changed: 921 additions & 159 deletions

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,11 @@ sim files delete wf_8Kd2NpVrY6zTfQa3XwBmS --yes
8989
Without `--yes` the command explains what it would have destroyed and stops.
9090

9191
<Callout type="warn">
92-
`batch-delete` and `batch-update` carry the default `--limit` of `100`, so a
93-
filter matching more rows than that silently affects only the first 100. Pass
94-
`--limit 0` to affect every matching row.
92+
On `batch-delete` and `batch-update`, `--limit` has no default and is not a page
93+
size — it is a ceiling on how many matching rows the one call may touch. Leave it
94+
off and the command acts on **every** row the filter matches, however many that
95+
is. `--limit 0` is not the unbounded form here and is rejected; pass a whole
96+
number of 1 or more to cap the blast radius, or omit the flag deliberately.
9597
</Callout>
9698

9799
## Exit codes

apps/docs/openapi-v2-knowledge.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3481,7 +3481,7 @@
34813481
"patch": {
34823482
"operationId": "bulkUpdateKnowledgeChunks",
34833483
"summary": "Bulk Update Chunks",
3484-
"description": "Enable, disable, or delete many chunks of one document in a single request. Best-effort: an identifier naming no chunk in the document is skipped rather than failing the request, so `processed` is the authoritative count. Chunks of a connector-synced document are read-only and a write answers `403` with `error.details.code: \"CONNECTOR_MANAGED_RESOURCE_READ_ONLY\"` — change the content at the source and re-sync, or exclude the document from the connector. A workspace API key is rejected with `403`; use a personal API key.",
3484+
"description": "Enable, disable, or delete many chunks of one document in a single request. Best-effort: an identifier naming no chunk in the document is reported in `errors` rather than failing the request. `processed` counts the chunks the operation matched, not the chunks it changed. Chunks of a connector-synced document are read-only and a write answers `403` with `error.details.code: \"CONNECTOR_MANAGED_RESOURCE_READ_ONLY\"` — change the content at the source and re-sync, or exclude the document from the connector. A workspace API key is rejected with `403`; use a personal API key.",
34853485
"tags": ["Knowledge Bases"],
34863486
"parameters": [
34873487
{

apps/docs/openapi-v2-logs.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@
421421
"get": {
422422
"operationId": "getLogStats",
423423
"summary": "Get Log Statistics",
424-
"description": "Bucketed run counts, success rate, error count, and mean latency for a workspace and for each of its workflows — the aggregate a caller would otherwise have to page every run to compute. The window spans `startDate` through `endDate` when both are supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs only the left edge falls back, to 24 hours before that right edge — the trailing 24 hours when neither edge was supplied, and the 24 hours preceding `endDate` when only `endDate` was supplied. A supplied `startDate` is still used verbatim, so a `startDate` without an `endDate` yields `[startDate, now]`, which can be any width. The window is divided into exactly `segmentCount` equal buckets whose width is `max(60000, floor(windowMs / segmentCount))` milliseconds. The one-minute floor is a floor on bucket width, not on the window: when it applies, the series runs past `timeBounds.end` and the trailing buckets are empty rather than the window being compressed. A folder path covers its whole subtree. Per-workflow series are capped and `workflowsTruncated` reports whether the cap applied; the workspace totals are always computed from every workflow. Runs are hard-deleted once they pass the payer's log retention window, so an older run is simply absent rather than reported as removed. The window is 30 days from run start on the free plan, unbounded on Pro and Team, and set per organization on Enterprise with an optional per-workspace override. A workspace folder tree over 10,000 folders is a `413`.",
424+
"description": "Bucketed run counts, success rate, error count, and mean latency for a workspace and for each of its workflows — the aggregate a caller would otherwise have to page every run to compute. The window spans `startDate` through `endDate` when both are supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs the right edge falls back to now and the left to 24 hours before that right edge — the trailing 24 hours when neither edge was supplied, and the 24 hours preceding `endDate` when only `endDate` was supplied. A supplied `startDate` is still used verbatim, so a `startDate` without an `endDate` yields `[startDate, now]`, which can be any width. The window is divided into exactly `segmentCount` equal buckets whose width is `max(60000, floor(windowMs / segmentCount))` milliseconds. The one-minute floor is a floor on bucket width, not on the window: when it applies, the series runs past `timeBounds.end` and the trailing buckets are empty rather than the window being compressed. A folder path covers its whole subtree. Per-workflow series are capped and `workflowsTruncated` reports whether the cap applied; the workspace totals are always computed from every workflow. Runs are hard-deleted once they pass the payer's log retention window, so an older run is simply absent rather than reported as removed. The window is 30 days from run start on the free plan, unbounded on Pro and Team, and set per organization on Enterprise with an optional per-workspace override. A workspace folder tree over 10,000 folders is a `413`.",
425425
"tags": ["Logs"],
426426
"parameters": [
427427
{
@@ -1779,7 +1779,7 @@
17791779
},
17801780
"required": ["start", "end"],
17811781
"additionalProperties": false,
1782-
"description": "The window the buckets span. `startDate` and `endDate` are used verbatim when supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs only the left edge falls back, to 24 hours before that right edge — the trailing 24 hours when neither edge was supplied, and the 24 hours preceding `endDate` when only `endDate` was supplied. A supplied `startDate` is still used verbatim, so a `startDate` without an `endDate` yields `[startDate, now]`, which can be any width."
1782+
"description": "The window the buckets span. `startDate` and `endDate` are used verbatim when supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs the right edge falls back to now and the left to 24 hours before that right edge — the trailing 24 hours when neither edge was supplied, and the 24 hours preceding `endDate` when only `endDate` was supplied. A supplied `startDate` is still used verbatim, so a `startDate` without an `endDate` yields `[startDate, now]`, which can be any width."
17831783
},
17841784
"segmentMs": {
17851785
"type": "number",

apps/docs/openapi-v2-workflows.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@
437437
"post": {
438438
"operationId": "applyWorkflowOperations",
439439
"summary": "Apply Workflow Operations",
440-
"description": "Apply a batch of semantic edits — add, edit, delete, and subflow membership changes — to a workflow graph, plus an optional set of block enable/disable changes.\n\nBest-effort per operation, atomic per write. The engine applies what it can to an in-memory graph and reports the rest in `skipped`, each with a machine-readable `type`; exactly one write of the fully-resolved graph then happens, so there is never a partially-applied graph. `deferred` is **not** a failure list: a forward-referencing edge is wired automatically once its target block exists, in this batch or a later one, so re-issuing a deferred edge is wrong.\n\nSet `atomic` to fail closed: any genuine skipped item, or any block input that would be dropped rather than persisted, then aborts before the write and answers `409` with `error.details.code: \"OPERATIONS_NOT_APPLIED\"`, the same `skipped` array, and a `droppedInputs` array, having persisted nothing.\n\nA `block_id` you supply on an `add` or `insert_into_subflow` is only a label unless it is already a UUID: the engine mints one and returns the pairing in `mintedBlockIds`. References between operations in the same batch are remapped for you, so `triage` can be wired up in the same call it is created in — but a later request must use the minted id. Send your own UUIDs when you want an id you chose to survive across requests.\n\nOperation `params` is an open object because the accepted inputs come from the block registry, not from this contract — see the per-operation schemas for the envelope: `inputs` keyed by sub-block id, with `retry`, `triggerMode` and `advancedMode` beside it rather than inside it, and `connections` keyed by source handle. `GET /blocks/{blockId}` publishes the inputs a given block type accepts. The Agent block’s `inputs.tools` value is the important exception to that open catalog shape: it is published here as the named `AgentToolInput` union, covering catalog integrations, workspace custom tools, and MCP tools.\n\n`lint` is advisory and never blocks the write. `lint.fieldIssues` is the most actionable part for a headless builder — it names blocks missing a required field, which fail at run time — and `lint.unresolvedReferences` names credential, resource, tool, and skill values that do not resolve. Those values stay persisted; only `inputValidationErrors` lists inputs that were actually dropped.\n\nAs with `PUT /workflows/{workflowId}/state`, this changes only the draft; deploy to publish it. A workspace API key is rejected with `403`; use a personal API key.\n\nSet `?dryRun=true` to validate and lint without persisting: nothing is written, no audit entry is recorded, and collaborators are not notified. The response carries the same shape and the same validation and `lint` findings the committed write would, with `dryRun: true` — including the same `409` when an id is already owned by another workflow. `needsRedeployment` describes the state before the write, and warnings raised by persistence itself are not reported.",
440+
"description": "Apply a batch of semantic edits — add, edit, delete, and subflow membership changes — to a workflow graph, plus an optional set of block enable/disable changes.\n\nBest-effort per operation, atomic per write. The engine applies what it can to an in-memory graph and reports the rest in `skipped`, each with a machine-readable `type`; exactly one write of the fully-resolved graph then happens, so there is never a partially-applied graph. `deferred` is **not** a failure list: a forward-referencing edge is wired automatically once its target block exists, in this batch or a later one, so re-issuing a deferred edge is wrong.\n\nSet `atomic` to fail closed: any genuine skipped item, or any block input that would be dropped rather than persisted, then aborts before the write and answers `409` with `error.details.code: \"OPERATIONS_NOT_APPLIED\"`, the same `skipped` array, and a `droppedInputs` array, having persisted nothing.\n\nA `block_id` you supply on an `add` or `insert_into_subflow` is only a label unless it is already a UUID: the engine mints one and returns the pairing in `mintedBlockIds`. References between operations in the same batch are remapped for you, so `triage` can be wired up in the same call it is created in — but a later request must use the minted id. Send your own UUIDs when you want an id you chose to survive across requests.\n\nOperation `params` is an open object because the accepted inputs come from the block registry, not from this contract — see the per-operation schemas for the envelope: `inputs` keyed by sub-block id, with `retry`, `triggerMode` and `advancedMode` beside it rather than inside it, and `connections` keyed by source handle. `GET /blocks/{blockId}` publishes the inputs a given block type accepts. The Agent block’s `inputs.tools` value is the important exception to that open catalog shape: it is published here as the named `AgentToolInput` union, covering catalog integrations, workspace custom tools, and MCP tools.\n\n`lint` is advisory and never blocks the write. `lint.fieldIssues` is the most actionable part for a headless builder — it names blocks missing a required field, which fail at run time — and `lint.unresolvedReferences` names credential, resource, tool, and skill values that do not resolve. Those values stay persisted; only `inputValidationErrors` lists inputs that were actually dropped.\n\nAs with `PUT /workflows/{workflowId}/state`, this changes only the draft; deploy to publish it. A workspace API key is rejected with `403`; use a personal API key.\n\nSet `?dryRun=true` to validate and lint without persisting: nothing is written, no audit entry is recorded, and collaborators are not notified. The response carries the same shape and the same validation and `lint` findings the committed write would, with `dryRun: true` — including the warnings the write’s own preparation step raises, and the same `409` when an id is already owned by another workflow. Only `needsRedeployment` differs: it describes the state before the write.",
441441
"tags": ["Workflows"],
442442
"parameters": [
443443
{

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

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const {
1414
mockCheckOperationRate,
1515
mockCheckPreAuthRate,
1616
mockGenerateId,
17+
mockPersistCopilotChatTurn,
1718
mockRequestExplicitStreamAbort,
1819
mockResolveBillingAttribution,
1920
mockResolveOrCreateChat,
@@ -35,6 +36,7 @@ const {
3536
mockCheckOperationRate: vi.fn(),
3637
mockCheckPreAuthRate: vi.fn(),
3738
mockGenerateId: vi.fn(),
39+
mockPersistCopilotChatTurn: vi.fn(),
3840
mockResolveBillingAttribution: vi.fn(),
3941
mockResolveOrCreateChat: vi.fn(),
4042
mockRequestExplicitStreamAbort: vi.fn().mockResolvedValue(undefined),
@@ -84,6 +86,10 @@ vi.mock('@/lib/copilot/chat/lifecycle', () => ({
8486
resolveOrCreateChat: mockResolveOrCreateChat,
8587
}))
8688

89+
vi.mock('@/lib/copilot/chat/messages-store', () => ({
90+
persistCopilotChatTurn: mockPersistCopilotChatTurn,
91+
}))
92+
8793
vi.mock('@/lib/copilot/chat/payload', () => ({
8894
buildIntegrationToolSchemas: vi.fn().mockResolvedValue([{ name: 'run_workflow' }]),
8995
}))
@@ -131,6 +137,7 @@ function chatRow(id: string) {
131137
const successResult = {
132138
success: true,
133139
content: 'Hello there',
140+
contentBlocks: [],
134141
toolCalls: [{ name: 'run_workflow' }, { name: 'internal_only' }],
135142
usage: { prompt: 10, completion: 5 },
136143
cost: { total: 0.01 },
@@ -160,6 +167,7 @@ describe('POST /api/v2/chat', () => {
160167
mockAssertActiveWorkspaceAccess.mockResolvedValue({ permission: 'admin' })
161168
mockResolveBillingAttribution.mockResolvedValue(billingAttributionSnapshot)
162169
mockRequestExplicitStreamAbort.mockResolvedValue(undefined)
170+
mockPersistCopilotChatTurn.mockResolvedValue(undefined)
163171
mockRunHeadlessCopilotLifecycle.mockResolvedValue(successResult)
164172
mockResolveOrCreateChat.mockResolvedValue({
165173
chatId: SERVER_ISSUED_CHAT_ID,
@@ -297,6 +305,33 @@ describe('POST /api/v2/chat', () => {
297305
})
298306
})
299307

308+
it('posts only the current turn on a resumed conversation, never the stored transcript', async () => {
309+
mockResolveOrCreateChat.mockResolvedValue({
310+
chatId: OWNED_CONVERSATION_ID,
311+
chat: chatRow(OWNED_CONVERSATION_ID),
312+
conversationHistory: [
313+
{ role: 'user', content: 'first' },
314+
{ role: 'assistant', content: 'first reply' },
315+
],
316+
isNew: false,
317+
})
318+
319+
const response = await callChat({
320+
workspaceId: 'workspace-1',
321+
message: 'and then?',
322+
conversationId: OWNED_CONVERSATION_ID,
323+
})
324+
325+
expect(response.status).toBe(200)
326+
// Continuity is keyed by chatId downstream, exactly as the web send path
327+
// and the Sim Chat block do. Replaying the transcript here would duplicate
328+
// every prior turn.
329+
expect(mockRunHeadlessCopilotLifecycle.mock.calls[0][0]).toMatchObject({
330+
messages: [{ role: 'user', content: 'and then?' }],
331+
chatId: OWNED_CONVERSATION_ID,
332+
})
333+
})
334+
300335
it('answers 404 and runs nothing when the resolver refuses the named conversation', async () => {
301336
mockResolveOrCreateChat.mockResolvedValue({
302337
chatId: OWNED_CONVERSATION_ID,
@@ -419,7 +454,11 @@ describe('POST /api/v2/chat', () => {
419454
})
420455

421456
it('ends the NDJSON stream with an error event when the run fails', async () => {
422-
mockRunHeadlessCopilotLifecycle.mockResolvedValue({ success: false, error: 'model exploded' })
457+
mockRunHeadlessCopilotLifecycle.mockResolvedValue({
458+
success: false,
459+
error: 'model exploded',
460+
contentBlocks: [],
461+
})
423462

424463
const response = await callChat(
425464
{ workspaceId: 'workspace-1', message: 'hi' },
@@ -432,4 +471,54 @@ describe('POST /api/v2/chat', () => {
432471
expect(last.type).toBe('error')
433472
expect(last.error).toBe('model exploded')
434473
})
474+
475+
it('persists both sides of the turn so the conversation is not an empty transcript', async () => {
476+
await callChat({ workspaceId: 'workspace-1', message: 'hi' })
477+
478+
expect(mockPersistCopilotChatTurn).toHaveBeenCalledTimes(1)
479+
const [chatId, messages] = mockPersistCopilotChatTurn.mock.calls[0]
480+
expect(chatId).toBe(SERVER_ISSUED_CHAT_ID)
481+
expect(messages.map((m: { role: string; content: string }) => [m.role, m.content])).toEqual([
482+
['user', 'hi'],
483+
['assistant', 'Hello there'],
484+
])
485+
})
486+
487+
it('persists the turn on the NDJSON path too, before the final event', async () => {
488+
const response = await callChat(
489+
{ workspaceId: 'workspace-1', message: 'hi' },
490+
{ accept: 'application/x-ndjson' }
491+
)
492+
const events = await readNdjsonEvents(response)
493+
494+
expect(mockPersistCopilotChatTurn).toHaveBeenCalledTimes(1)
495+
expect(mockPersistCopilotChatTurn.mock.calls[0][1]).toHaveLength(2)
496+
expect(events.at(-1)?.type).toBe('final')
497+
})
498+
499+
it('persists nothing when the run fails, so no question is stored without its answer', async () => {
500+
mockRunHeadlessCopilotLifecycle.mockResolvedValue({
501+
success: false,
502+
error: 'model exploded',
503+
contentBlocks: [],
504+
})
505+
506+
const response = await callChat({ workspaceId: 'workspace-1', message: 'hi' })
507+
508+
expect(response.status).toBe(500)
509+
expect(mockPersistCopilotChatTurn).not.toHaveBeenCalled()
510+
})
511+
512+
it('still answers the caller when persisting the transcript fails', async () => {
513+
mockPersistCopilotChatTurn.mockRejectedValue(new Error('transcript write failed'))
514+
515+
const response = await callChat({ workspaceId: 'workspace-1', message: 'hi' })
516+
517+
expect(response.status).toBe(200)
518+
const body = await response.json()
519+
expect(body.data).toMatchObject({
520+
content: 'Hello there',
521+
conversationId: SERVER_ISSUED_CHAT_ID,
522+
})
523+
})
435524
})

0 commit comments

Comments
 (0)