mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-21 13:00:04 +08:00
fix(v2): derive the log and run status enums from the persisted status list (#6612)
* fix(v2): derive the log and run status enums from the persisted status list
`GET /api/v2/logs` and `GET /api/v2/logs/{runId}` parse the raw
`workflow_execution_logs.status` column against a six-value enum that omits
`paused`, so a run holding that value returns 500. The list response is
validated whole-page, so one such row 500s every page it lands on, and the
row is durable until the run is resumed, cancelled, or failed.
`paused` is not written by an ordinary human-in-the-loop pause — that path
persists `pending` (logging-session.ts:1180). It is written by
`PauseResumeManager.markResumeAttemptFailed`, which fires on any
`ResumeAdmissionError`: a workspace over its usage limit, an archived or
undeployed workflow, or a concurrent resume losing the claim race. That is a
routine business path.
The enum was supposed to be protected by an `AssertNever` exhaustiveness gate,
but the gate was vacuous: it compared against `PersistedWorkflowExecutionStatus`,
a hand-written union that was itself missing `paused`, because the write goes
through a raw `sql` CASE fragment Drizzle cannot type-check. Adding `paused` to
both lists would leave the same vacuous gate in place for the next status.
Instead, `PERSISTED_WORKFLOW_EXECUTION_STATUSES` becomes the single runtime
source of truth, `PersistedWorkflowExecutionStatus` is derived from it, and both
v2 contracts derive their enums from the const rather than re-declaring them.
Both surfaces pass the column through verbatim, so their reported set is the
persisted set by definition — there is no editorial choice for a gate to force,
only the question of whether a newly persisted status should be public, which
the option-list tests now pin. The `[...V2_PERSISTED_RUN_STATUSES, 'paused']`
append on the runs contract is deleted rather than adjusted; it would otherwise
be a duplicate.
Alternatives rejected:
- A `.catch()` or `safeParse` in the presenters is dead code:
`v2-json-route.ts:271` re-parses the whole body with the same schema.
- Normalizing `markResumeAttemptFailed` to write `pending` would remove the
distinction the resume claim query at human-in-the-loop-manager.ts:973 relies
on, and leaves the contract wrong for any other future status.
- Typing the Drizzle column does not help: the offending write is a raw `sql`
fragment, and `packages/db` cannot import the app's status list.
The v2 workflows spec changes are reordering and description only — the value
set there already contained `paused`. The v2 logs spec gains `paused`, which is
additive and safe while the whole `/api/v2` surface is behind the off-by-default
`v2-api` flag; it must land before v2 GA, after which it would be breaking.
* fix(v2): document both provenances of a reported paused run status
* fix(v2): stop promising a paused discriminator the response cannot always provide
* fix(v2): describe the paused discriminator as the code actually records it
This commit is contained in:
Waleed
@@ -640,8 +640,16 @@
|
||||
},
|
||||
"status": {
|
||||
"type": "string",
|
||||
"enum": ["pending", "running", "redacting", "completed", "failed", "cancelled"],
|
||||
"description": "Current execution status. `redacting` is transient while run output is scrubbed."
|
||||
"enum": [
|
||||
"pending",
|
||||
"running",
|
||||
"paused",
|
||||
"redacting",
|
||||
"completed",
|
||||
"failed",
|
||||
"cancelled"
|
||||
],
|
||||
"description": "Current execution status. `redacting` is transient while run output is scrubbed. `paused` is reported when a resume attempt did not run to completion and the run is waiting to be resumed again."
|
||||
},
|
||||
"level": {
|
||||
"type": "string",
|
||||
@@ -1028,8 +1036,16 @@
|
||||
},
|
||||
"status": {
|
||||
"type": "string",
|
||||
"enum": ["pending", "running", "redacting", "completed", "failed", "cancelled"],
|
||||
"description": "Current execution status. `redacting` is transient while run output is scrubbed."
|
||||
"enum": [
|
||||
"pending",
|
||||
"running",
|
||||
"paused",
|
||||
"redacting",
|
||||
"completed",
|
||||
"failed",
|
||||
"cancelled"
|
||||
],
|
||||
"description": "Current execution status. `redacting` is transient while run output is scrubbed. `paused` is reported when a resume attempt did not run to completion and the run is waiting to be resumed again."
|
||||
},
|
||||
"level": {
|
||||
"type": "string",
|
||||
|
||||
@@ -3923,13 +3923,13 @@
|
||||
"enum": [
|
||||
"pending",
|
||||
"running",
|
||||
"paused",
|
||||
"redacting",
|
||||
"completed",
|
||||
"failed",
|
||||
"cancelled",
|
||||
"paused"
|
||||
"cancelled"
|
||||
],
|
||||
"description": "Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed."
|
||||
"description": "Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed. `paused` means the run is not executing and is waiting to be resumed: either held at a human-in-the-loop pause point, or left paused because a resume attempt did not run to completion. The status alone does not say which. On the single-run response `paused.automaticResumeWaitingReason` distinguishes them: it is recorded whenever a resume attempt fails and cleared once a resume succeeds, so a null value means the run is waiting on human input. When the failure is not retryable or the automatic retries are exhausted, the reason is prefixed `Automatic resume requires manual intervention: `. Run-list items carry no `paused` object, so the two cases are indistinguishable there."
|
||||
},
|
||||
"trigger": {
|
||||
"type": "string",
|
||||
@@ -4063,14 +4063,14 @@
|
||||
"enum": [
|
||||
"pending",
|
||||
"running",
|
||||
"paused",
|
||||
"redacting",
|
||||
"completed",
|
||||
"failed",
|
||||
"cancelled",
|
||||
"paused",
|
||||
"queued"
|
||||
],
|
||||
"description": "Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed."
|
||||
"description": "Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed. `paused` means the run is not executing and is waiting to be resumed: either held at a human-in-the-loop pause point, or left paused because a resume attempt did not run to completion. The status alone does not say which. On the single-run response `paused.automaticResumeWaitingReason` distinguishes them: it is recorded whenever a resume attempt fails and cleared once a resume succeeds, so a null value means the run is waiting on human input. When the failure is not retryable or the automatic retries are exhausted, the reason is prefixed `Automatic resume requires manual intervention: `. Run-list items carry no `paused` object, so the two cases are indistinguishable there."
|
||||
},
|
||||
"trigger": {
|
||||
"anyOf": [
|
||||
|
||||
@@ -94,6 +94,21 @@ describe('GET /api/v2/logs/[runId]', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('serves a run whose persisted status is paused', async () => {
|
||||
mocks.execute.mockResolvedValue({
|
||||
log: { ...log, status: 'paused' },
|
||||
workflowFolderPath: '/agents',
|
||||
executionData: { traceSpans: [], finalOutput: null },
|
||||
})
|
||||
|
||||
const response = await GET(new NextRequest('http://localhost:3000/api/v2/logs/run-1'), {
|
||||
params: Promise.resolve({ runId: 'run-1' }),
|
||||
})
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
expect((await response.json()).data).toMatchObject({ runId: 'run-1', status: 'paused' })
|
||||
})
|
||||
|
||||
it('conceals canonical workspace authorization as log not-found', async () => {
|
||||
mocks.execute.mockRejectedValueOnce(new NoWorkspaceAccessError())
|
||||
|
||||
|
||||
@@ -98,6 +98,24 @@ describe('GET /api/v2/logs', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('serves a run whose persisted status is paused', async () => {
|
||||
mocks.execute.mockResolvedValue({
|
||||
items: [{ log: { ...log, status: 'paused' }, executionData: null }],
|
||||
nextCursor: null,
|
||||
includeFullDetails: false,
|
||||
includeFinalOutput: false,
|
||||
includeTraceSpans: false,
|
||||
})
|
||||
|
||||
const response = await GET(
|
||||
new NextRequest(`http://localhost:3000/api/v2/logs?workspaceId=${WORKSPACE_ID}`)
|
||||
)
|
||||
const body = await response.json()
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
expect(body.data[0]).toMatchObject({ runId: 'run-1', status: 'paused' })
|
||||
})
|
||||
|
||||
it('rejects malformed cursors after admission and before protected reads', async () => {
|
||||
const response = await GET(
|
||||
new NextRequest(
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { v2LogStatusSchema } from '@/lib/api/contracts/v2/logs'
|
||||
import { PERSISTED_WORKFLOW_EXECUTION_STATUSES } from '@/lib/logs/types'
|
||||
|
||||
/**
|
||||
* Both log endpoints pass `workflow_execution_logs.status` through verbatim — unlike the
|
||||
* run endpoints there is no `paused` overlay and no `queued` — so any drift between the
|
||||
* reported enum and the persisted list 500s a whole page of results.
|
||||
*/
|
||||
describe('v2 log status schema', () => {
|
||||
it('publishes exactly the persisted statuses', () => {
|
||||
expect(v2LogStatusSchema.options).toEqual([
|
||||
'pending',
|
||||
'running',
|
||||
'paused',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
])
|
||||
})
|
||||
|
||||
it('stays derived from the persisted status list', () => {
|
||||
expect(v2LogStatusSchema.options).toEqual([...PERSISTED_WORKFLOW_EXECUTION_STATUSES])
|
||||
})
|
||||
})
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
v2FolderPathSchema,
|
||||
v2TimestampSchema,
|
||||
} from '@/lib/api/contracts/v2/shared'
|
||||
import type { PersistedWorkflowExecutionStatus } from '@/lib/logs/types'
|
||||
import { PERSISTED_WORKFLOW_EXECUTION_STATUSES } from '@/lib/logs/types'
|
||||
|
||||
/**
|
||||
* v2 logs contracts. The query schemas are reused verbatim from v1 (the request
|
||||
@@ -23,29 +23,16 @@ const v2LogCostSchema = z
|
||||
.nullable()
|
||||
.describe('Cost charged for the run, or null when unavailable.')
|
||||
/**
|
||||
* Every status the execution logger can persist, including the transient
|
||||
* `redacting` state written while a finished run's output is scrubbed. The
|
||||
* column is free text, so a value missing here fails the response parse and
|
||||
* turns a single row into a 500 for the whole page. `_ExhaustiveLogStatus`
|
||||
* makes a future addition to the persisted union a compile error instead.
|
||||
* Both log endpoints pass `workflow_execution_logs.status` through verbatim, so the
|
||||
* reported set is exactly the persisted set — a value missing here fails the response
|
||||
* parse, and because list validation is whole-page one such row turns an entire page
|
||||
* into a 500.
|
||||
*/
|
||||
const V2_LOG_STATUSES = [
|
||||
'pending',
|
||||
'running',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
] as const satisfies readonly PersistedWorkflowExecutionStatus[]
|
||||
|
||||
type AssertNever<T extends never> = T
|
||||
type _ExhaustiveLogStatus = AssertNever<
|
||||
Exclude<PersistedWorkflowExecutionStatus, (typeof V2_LOG_STATUSES)[number]>
|
||||
>
|
||||
|
||||
export const v2LogStatusSchema = z
|
||||
.enum(V2_LOG_STATUSES)
|
||||
.describe('Current execution status. `redacting` is transient while run output is scrubbed.')
|
||||
.enum(PERSISTED_WORKFLOW_EXECUTION_STATUSES)
|
||||
.describe(
|
||||
'Current execution status. `redacting` is transient while run output is scrubbed. `paused` is reported when a resume attempt did not run to completion and the run is waiting to be resumed again.'
|
||||
)
|
||||
|
||||
/** Execution `files` is a per-run jsonb array of attachment metadata. */
|
||||
const v2LogFilesSchema = z
|
||||
|
||||
@@ -4,36 +4,49 @@ import {
|
||||
v2WorkflowRunStatusFilterSchema,
|
||||
v2WorkflowRunStatusValueSchema,
|
||||
} from '@/lib/api/contracts/v2/workflows'
|
||||
import type { PersistedWorkflowExecutionStatus } from '@/lib/logs/types'
|
||||
import { PERSISTED_WORKFLOW_EXECUTION_STATUSES } from '@/lib/logs/types'
|
||||
|
||||
/**
|
||||
* The runtime mirror of the persisted union. `satisfies` keeps it honest against
|
||||
* `PersistedWorkflowExecutionStatus`, and the `AssertNever` gate in the contract keeps
|
||||
* that union honest against the reported enums, so a status added to the execution logger
|
||||
* fails compilation in both places before it can 500 a response parse.
|
||||
* Both run endpoints report `workflow_execution_logs.status`, overlaid with `paused` from
|
||||
* `paused_executions`, so every reported value lands in the persisted set. These tests
|
||||
* guard the two ways that can break: the derivation being replaced by a hand-maintained
|
||||
* list again, and a status being added to the persisted set without anyone confirming it
|
||||
* belongs on the public wire (and regenerating the OpenAPI specs).
|
||||
*/
|
||||
const PERSISTED_STATUSES = [
|
||||
'pending',
|
||||
'running',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
] as const satisfies readonly PersistedWorkflowExecutionStatus[]
|
||||
|
||||
describe('v2 workflow run status schemas', () => {
|
||||
it.each(PERSISTED_STATUSES)('reports the persisted status %s on both run endpoints', (status) => {
|
||||
expect(v2WorkflowRunListStatusValueSchema.parse(status)).toBe(status)
|
||||
expect(v2WorkflowRunStatusValueSchema.parse(status)).toBe(status)
|
||||
it('publishes exactly the persisted statuses on the run list', () => {
|
||||
expect(v2WorkflowRunListStatusValueSchema.options).toEqual([
|
||||
'pending',
|
||||
'running',
|
||||
'paused',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
])
|
||||
})
|
||||
|
||||
it('reports the paused overlay on both run endpoints', () => {
|
||||
expect(v2WorkflowRunListStatusValueSchema.parse('paused')).toBe('paused')
|
||||
expect(v2WorkflowRunStatusValueSchema.parse('paused')).toBe('paused')
|
||||
it('stays derived from the persisted status list', () => {
|
||||
expect(v2WorkflowRunListStatusValueSchema.options).toEqual([
|
||||
...PERSISTED_WORKFLOW_EXECUTION_STATUSES,
|
||||
])
|
||||
expect(v2WorkflowRunStatusValueSchema.options).toEqual([
|
||||
...PERSISTED_WORKFLOW_EXECUTION_STATUSES,
|
||||
'queued',
|
||||
])
|
||||
})
|
||||
|
||||
it('reports queued only where the job queue is consulted', () => {
|
||||
expect(v2WorkflowRunStatusValueSchema.parse('queued')).toBe('queued')
|
||||
expect(v2WorkflowRunStatusValueSchema.options).toEqual([
|
||||
'pending',
|
||||
'running',
|
||||
'paused',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
'queued',
|
||||
])
|
||||
expect(v2WorkflowRunListStatusValueSchema.safeParse('queued').success).toBe(false)
|
||||
})
|
||||
|
||||
|
||||
@@ -36,7 +36,7 @@ import {
|
||||
workflowIdParamsSchema,
|
||||
} from '@/lib/api/contracts/workflows'
|
||||
import { MAX_WORKFLOW_EXECUTION_TIMEOUT_SECONDS } from '@/lib/billing/execution-timeout-defaults'
|
||||
import type { PersistedWorkflowExecutionStatus } from '@/lib/logs/types'
|
||||
import { PERSISTED_WORKFLOW_EXECUTION_STATUSES } from '@/lib/logs/types'
|
||||
|
||||
export const V2_WORKFLOW_RUN_ID_HEADER = 'X-Run-Id'
|
||||
|
||||
@@ -982,40 +982,20 @@ export const v2ResumeWorkflowContract = defineRouteContract({
|
||||
},
|
||||
})
|
||||
|
||||
/**
|
||||
* Every status the execution logger can persist into `workflow_execution_logs.status`,
|
||||
* including the transient `redacting` state written while a finished run's output is
|
||||
* scrubbed. The column is free text and both run endpoints pass it straight through, so
|
||||
* a value missing here fails the response parse — and because list validation is
|
||||
* whole-page, one such row turns an entire page into a 500. `_ExhaustiveRunStatus` makes
|
||||
* a future addition to the persisted union a compile error instead.
|
||||
*/
|
||||
const V2_PERSISTED_RUN_STATUSES = [
|
||||
'pending',
|
||||
'running',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
] as const satisfies readonly PersistedWorkflowExecutionStatus[]
|
||||
|
||||
type AssertNever<T extends never> = T
|
||||
type _ExhaustiveRunStatus = AssertNever<
|
||||
Exclude<PersistedWorkflowExecutionStatus, (typeof V2_PERSISTED_RUN_STATUSES)[number]>
|
||||
>
|
||||
|
||||
/**
|
||||
* The list projection overlays `paused` onto the persisted status whenever the run has a
|
||||
* `paused` or `partially_resumed` row in `paused_executions`. It cannot report `queued`:
|
||||
* a run that is still only in the job queue has no log row to list.
|
||||
*/
|
||||
const V2_WORKFLOW_RUN_LIST_STATUSES = [...V2_PERSISTED_RUN_STATUSES, 'paused'] as const
|
||||
|
||||
const RUN_STATUS_DESCRIPTION =
|
||||
'Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed.'
|
||||
'Current or terminal run status. `redacting` is transient, reported while the output of a finished run is being scrubbed. `paused` means the run is not executing and is waiting to be resumed: either held at a human-in-the-loop pause point, or left paused because a resume attempt did not run to completion. The status alone does not say which. On the single-run response `paused.automaticResumeWaitingReason` distinguishes them: it is recorded whenever a resume attempt fails and cleared once a resume succeeds, so a null value means the run is waiting on human input. When the failure is not retryable or the automatic retries are exhausted, the reason is prefixed `Automatic resume requires manual intervention: `. Run-list items carry no `paused` object, so the two cases are indistinguishable there.'
|
||||
|
||||
/**
|
||||
* The list projection passes `workflow_execution_logs.status` through except where it
|
||||
* overlays `paused` for a run holding a `paused` or `partially_resumed` row in
|
||||
* `paused_executions` — so a reported `paused` is either that overlay or the persisted
|
||||
* value a failed resume attempt left behind. Both branches land in the persisted set, so the reported enum is
|
||||
* derived from it — a value missing here fails the response parse, and because list
|
||||
* validation is whole-page one such row turns an entire page into a 500. `queued` is not
|
||||
* reportable: a run still only in the job queue has no log row to list.
|
||||
*/
|
||||
export const v2WorkflowRunListStatusValueSchema = z
|
||||
.enum(V2_WORKFLOW_RUN_LIST_STATUSES)
|
||||
.enum(PERSISTED_WORKFLOW_EXECUTION_STATUSES)
|
||||
.describe(RUN_STATUS_DESCRIPTION)
|
||||
|
||||
/**
|
||||
@@ -1023,7 +1003,7 @@ export const v2WorkflowRunListStatusValueSchema = z
|
||||
* so a run accepted but not yet started reports `queued` rather than 404.
|
||||
*/
|
||||
export const v2WorkflowRunStatusValueSchema = z
|
||||
.enum([...V2_WORKFLOW_RUN_LIST_STATUSES, 'queued'])
|
||||
.enum([...PERSISTED_WORKFLOW_EXECUTION_STATUSES, 'queued'])
|
||||
.describe(RUN_STATUS_DESCRIPTION)
|
||||
|
||||
/**
|
||||
|
||||
@@ -204,13 +204,31 @@ export interface WorkflowExecutionLog {
|
||||
createdAt: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Every value written into `workflow_execution_logs.status`. The column is free text and
|
||||
* one writer sets it through a raw `sql` CASE Drizzle cannot type-check, so this list —
|
||||
* not the column type — is the only source of truth. API contracts that pass the column
|
||||
* through derive their enums from it, so adding a status here widens the public wire; the
|
||||
* contract tests fail until that widening is reviewed and the OpenAPI specs regenerated.
|
||||
*
|
||||
* `redacting` is transient while a finished run's output is scrubbed. `paused` is written
|
||||
* only by `PauseResumeManager.markResumeAttemptFailed`, when a resume attempt does not run
|
||||
* to completion — it failed admission, the run buffer was unavailable, the resume job could
|
||||
* not be enqueued, or the attempt was cancelled. An ordinary human-in-the-loop pause
|
||||
* persists `pending`.
|
||||
*/
|
||||
export const PERSISTED_WORKFLOW_EXECUTION_STATUSES = [
|
||||
'pending',
|
||||
'running',
|
||||
'paused',
|
||||
'redacting',
|
||||
'completed',
|
||||
'failed',
|
||||
'cancelled',
|
||||
] as const
|
||||
|
||||
export type PersistedWorkflowExecutionStatus =
|
||||
| 'running'
|
||||
| 'pending'
|
||||
| 'completed'
|
||||
| 'failed'
|
||||
| 'cancelled'
|
||||
| 'redacting'
|
||||
(typeof PERSISTED_WORKFLOW_EXECUTION_STATUSES)[number]
|
||||
|
||||
export interface CompletedWorkflowExecutionLog extends WorkflowExecutionLog {
|
||||
persistedStatus: PersistedWorkflowExecutionStatus
|
||||
|
||||
@@ -411,7 +411,8 @@ export const workflowExecutionLogs = pgTable(
|
||||
),
|
||||
|
||||
level: text('level').notNull(), // 'info' | 'error'
|
||||
status: text('status').notNull().default('running'), // 'running' | 'pending' | 'completed' | 'failed' | 'cancelled'
|
||||
/** See `PERSISTED_WORKFLOW_EXECUTION_STATUSES` in `apps/sim/lib/logs/types.ts`. */
|
||||
status: text('status').notNull().default('running'),
|
||||
trigger: text('trigger').notNull(), // 'api' | 'webhook' | 'schedule' | 'manual' | 'chat'
|
||||
|
||||
startedAt: timestamp('started_at').notNull(),
|
||||
|
||||
Reference in New Issue
Block a user