GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||
GeezerVison is showing github.com — fix(v2): derive the log and run status enums from the persisted status list by waleedlatif1 · Pull Request #6612 · simstudioai/sim Conversation
Collaborator
Summary
Verification
Type of Change
Testing14 tests across 4 suites: Checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
…s 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.
|
There was an error while loading. Please reload this page.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
PR SummaryMedium Risk Overview Introduces Reviewed by Cursor Bugbot for commit f6e0952. Configure here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Greptile SummaryThe PR establishes the persisted workflow-execution status tuple as the runtime source of truth for v2 log and run status schemas, allowing persisted
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/logs/types.ts | Introduces the canonical persisted-status tuple and derives the corresponding TypeScript union from it. |
| apps/sim/lib/api/contracts/v2/logs.ts | Derives the v2 log status schema from the persisted tuple so paused rows validate successfully. |
| apps/sim/lib/api/contracts/v2/workflows.ts | Derives run status schemas from the persisted tuple and accurately documents the limits of paused-state discrimination. |
| packages/db/schema.ts | Replaces stale inline status guidance with compliant TSDoc pointing to the canonical runtime status list. |
| apps/sim/lib/api/contracts/v2/log-status.test.ts | Pins the public log-status wire contract and verifies that it remains derived from the persisted tuple. |
| apps/sim/lib/api/contracts/v2/workflow-run-status.test.ts | Verifies persisted-status derivation while preserving the detail-only queued overlay and narrower filter contract. |
| apps/docs/openapi-v2-logs.json | Regenerates the log API specification to include and explain the persisted paused status. |
| apps/docs/openapi-v2-workflows.json | Regenerates workflow run schemas with the derived status ordering and corrected paused-state semantics. |
Reviews (3): Last reviewed commit: "fix(v2): stop promising a paused discrim..." | Re-trigger Greptile
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6931ea6. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 16ba04d. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f6e0952. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
4df9a7d1
Deployed
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.