chore: add slug workflow api endpoints#1137
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds slug support as a first-class workflow identifier across the API, introducing OpenAPI-documented endpoints for slug-based lookup and run creation, slug validation and conflict detection utilities, and comprehensive E2E test coverage to validate the new behavior. ChangesWorkflow slug API support
Sequence Diagram(s)sequenceDiagram
participant Client
participant createWorkflow as createWorkflow handler
participant Database
participant getWorkflowBySlug as getWorkflowBySlug handler
participant createWorkflowRunBySlug as createWorkflowRunBySlug handler
participant downstreamRun as downstream run endpoint
Client->>createWorkflow: POST slug, name
createWorkflow->>createWorkflow: resolveSlug(slug, name)
createWorkflow->>Database: insert with computed slug
Database-->>createWorkflow: conflict if slug exists
createWorkflow-->>Client: 409 DUPLICATE_SLUG
Client->>getWorkflowBySlug: GET by slug
getWorkflowBySlug->>Database: query (workspaceId, slug)
Database-->>getWorkflowBySlug: workflow or not found
getWorkflowBySlug-->>Client: 200 Workflow or 404
Client->>createWorkflowRunBySlug: POST /workflows/slug/:slug/runs
createWorkflowRunBySlug->>Database: lookup workflow by slug
createWorkflowRunBySlug->>downstreamRun: proxy to downstream with workflowId
downstreamRun-->>createWorkflowRunBySlug: WorkflowRun
createWorkflowRunBySlug-->>Client: 201 WorkflowRun
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds slug as a first-class workflow identifier in the API: a new lookup-by-slug endpoint, a run-by-slug endpoint, slug validation/uniqueness handling on create and update, and corresponding e2e tests and OpenAPI/type-generation updates.
Changes:
- New routes
GET /workflows/slug/:slugandPOST /workflows/slug/:slug/runs, with shared slug validation and a 23505 → 409DUPLICATE_SLUGmapping. createWorkflownow derives slug from name (rejecting names that slugify to empty) or validates a user-supplied slug;updateWorkflowvalidates slug and surfaces conflicts.- OpenAPI artifacts (
workflows.jsonnet,openapi.json) and generated TS schemas (apps/api/src/types/openapi.ts,apps/web/app/api/openapi.ts,e2e/api/schema.ts) regenerated; new e2e tests cover happy paths, 404s, dup conflicts, invalid slugs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/src/routes/v1/workspaces/workflows.ts | Adds slug schema, resolveSlug, conflict mapping, by-slug GET/run handlers, and routes. |
| apps/api/openapi/paths/workflows.jsonnet | Adds by-slug paths and 409 responses for create/update. |
| apps/api/openapi/openapi.json | Regenerated spec with new endpoints and 409 responses. |
| apps/api/src/types/openapi.ts | Regenerated TS types matching the new spec. |
| apps/web/app/api/openapi.ts | Regenerated TS types for the web client; also picks up DeploymentVersionWithDependencies. |
| e2e/api/schema.ts | Regenerated e2e schema types. |
| e2e/tests/api/workflows.spec.ts | New e2e tests for by-slug get/run, dup conflict on create/update, invalid slug, empty-derived slug. |
Comments suppressed due to low confidence (1)
apps/api/src/routes/v1/workspaces/workflows.ts:193
- In
updateWorkflow, the slug-conflict catch is wired withreq.body.slug ?? "". If a future schema change introduces another unique constraint and produces a 23505 on update without a slug in the body, this would surface a misleading "Workflow slug '' already exists in this workspace" error. Consider only attaching the conflict handler whenreq.body.slugis actually being set, or re-throwing when slug is missing rather than substituting an empty string.
.then(takeFirstOrNull)
.catch((error) =>
throwOnSlugConflict(workspaceId, req.body.slug ?? "", error),
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .get("/slug/:slug", asyncHandler(getWorkflowBySlug)) | ||
| .post("/slug/:slug/runs", asyncHandler(createWorkflowRunBySlug)) | ||
| .get("/:workflowId", asyncHandler(getWorkflow)) | ||
| .put("/:workflowId", asyncHandler(updateWorkflow)) |
Comment on lines
+36
to
+42
| if (derived === "") | ||
| throw new ApiError( | ||
| "Could not derive slug from name; name must contain at least one alphanumeric character or pass an explicit slug", | ||
| 400, | ||
| "INVALID_SLUG", | ||
| ); | ||
| return derived; |
Comment on lines
+45
to
+67
| const throwOnSlugConflict = async ( | ||
| workspaceId: string, | ||
| slug: string, | ||
| error: unknown, | ||
| ) => { | ||
| if ((error as { code?: string }).code !== "23505") throw error; | ||
| const existing = await db | ||
| .select({ id: schema.workflow.id }) | ||
| .from(schema.workflow) | ||
| .where( | ||
| and( | ||
| eq(schema.workflow.workspaceId, workspaceId), | ||
| eq(schema.workflow.slug, slug), | ||
| ), | ||
| ) | ||
| .then(takeFirstOrNull); | ||
| throw new ApiError( | ||
| `Workflow slug '${slug}' already exists in this workspace`, | ||
| 409, | ||
| "DUPLICATE_SLUG", | ||
| { slug, existingWorkflowId: existing?.id }, | ||
| ); | ||
| }; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
e2e/tests/api/workflows.spec.ts (2)
218-257: ⚡ Quick winAssert conflict payload details, not only 409 status.
Both duplicate-slug tests should also verify the error body includes the conflicting
slugand existing workflow UUID, otherwise the key contract can regress undetected.Also applies to: 258-312
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tests/api/workflows.spec.ts` around lines 218 - 257, Update the duplicate-slug tests to assert the error response body contains the conflicting slug and the existing workflow UUID: after creating the first workflow (variables firstRes and firstId) and receiving dupRes, assert dupRes.data (or dupRes.response.body) includes slug === the generated slug and includes the existing workflow id (firstId) or a field like conflictingWorkflowId; make the same assertions in the other duplicate-slug test mentioned (lines 258-312) so the contract verifies both the 409 status and the error payload contains the conflicting slug and workflow UUID.
171-365: 🏗️ Heavy liftUse YAML entity fixtures for these E2E cases.
These new tests use imperative API setup/teardown instead of YAML fixtures with
importEntitiesFromYaml/cleanupImportedEntities, which diverges from the E2E test pattern and makes maintenance harder.As per coding guidelines, "E2E tests use YAML fixture files (
.spec.yamlalongside.spec.ts) to declare test entities; useimportEntitiesFromYamlto load them andcleanupImportedEntitiesto tear them down".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tests/api/workflows.spec.ts` around lines 171 - 365, Replace the imperative create/delete calls in the workflow E2E tests (tests like "should get a workflow by slug", "should reject creating a workflow with a duplicate slug in the same workspace", "should reject updating a workflow to a duplicate slug in the same workspace", etc.) with YAML entity fixtures: add corresponding .spec.yaml fixtures describing the workflows and slugs, load them at test start via importEntitiesFromYaml (using the same workspace context) and remove the api.POST/api.DELETE setup/teardown, and ensure tests call cleanupImportedEntities in afterEach/afterAll to tear down; update any tests that depend on dynamic slugs to reference the fixture entity names instead of creating them via api.POST.apps/api/openapi/paths/workflows.jsonnet (1)
64-81: ⚡ Quick winExtract duplicate request body schema into a reusable reference.
The request body schema for workflow run creation is duplicated between this slug-based endpoint (lines 68-78) and the ID-based endpoint (lines 151-161). Both define an identical inline schema with required
inputsobject. Extract this into a shared schema reference likeCreateWorkflowRunto maintain a single source of truth.♻️ Proposed refactor to eliminate duplication
First, define the schema in your schemas file (e.g.,
schemas.jsonnet):CreateWorkflowRun: { type: 'object', required: ['inputs'], properties: { inputs: { type: 'object', additionalProperties: true, description: 'Input values for the workflow run.', }, }, }Then, update both endpoints to reference the schema:
requestBody: { required: true, content: { 'application/json': { - schema: { - type: 'object', - required: ['inputs'], - properties: { - inputs: { - type: 'object', - additionalProperties: true, - description: 'Input values for the workflow run.', - }, - }, - }, + schema: openapi.schemaRef('CreateWorkflowRun'), }, }, },Apply the same change at lines 151-161 for the ID-based endpoint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/openapi/paths/workflows.jsonnet` around lines 64 - 81, Extract the duplicated inline requestBody schema into a shared schema named CreateWorkflowRun (an object with required ['inputs'] and inputs: {type: 'object', additionalProperties: true, description: 'Input values for the workflow run.'}), add it to your central schemas collection (e.g., schemas.jsonnet), and replace the inline requestBody.schema in both the slug-based endpoint and the ID-based endpoint with a reference to that shared schema (e.g., $ref or the equivalent jsonnet reference to CreateWorkflowRun) so both endpoints use a single source of truth.apps/api/src/routes/v1/workspaces/workflows.ts (1)
170-178: ⚡ Quick winExtract the explicit‑slug validation into a shared helper.
This block duplicates the validation branch inside
resolveSlug(Lines 24‑32). A small helper avoids drift between the create and update paths if the error message or schema changes later.♻️ Suggested refactor
+const validateExplicitSlug = (slug: string) => { + const result = slugSchema.safeParse(slug); + if (!result.success) + throw new ApiError( + `Invalid slug: ${result.error.issues[0]?.message ?? "invalid format"}`, + 400, + "INVALID_SLUG", + ); + return slug; +}; + const resolveSlug = (provided: string | undefined, name: string) => { - if (provided != null) { - const result = slugSchema.safeParse(provided); - if (!result.success) - throw new ApiError( - `Invalid slug: ${result.error.issues[0]?.message ?? "invalid format"}`, - 400, - "INVALID_SLUG", - ); - return provided; - } + if (provided != null) return validateExplicitSlug(provided); ... };And in
updateWorkflow:- if (req.body.slug != null) { - const result = slugSchema.safeParse(req.body.slug); - if (!result.success) - throw new ApiError( - `Invalid slug: ${result.error.issues[0]?.message ?? "invalid format"}`, - 400, - "INVALID_SLUG", - ); - } + if (req.body.slug != null) validateExplicitSlug(req.body.slug);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/v1/workspaces/workflows.ts` around lines 170 - 178, Extract the duplicate explicit-slug validation into a single helper (e.g., validateSlug or ensureValidSlug) and use it from both the inline check in this file and from resolveSlug so create and update paths share the same logic; the helper should accept the candidate slug (the value currently passed to slugSchema.safeParse) and, on validation failure, throw the same ApiError with the message built from result.error.issues[0]?.message ?? "invalid format" and code "INVALID_SLUG" so updateWorkflow and resolveSlug both call this helper instead of duplicating the slugSchema.safeParse block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/openapi/openapi.json`:
- Around line 9858-9866: The 409 responses currently reference the generic
components/schemas/ErrorResponse and must be replaced with a structured conflict
schema that includes the conflicting slug and existing workflow UUID (e.g.,
create a components/schemas/SlugConflictResponse or add a details shape with {
slug: string, existingWorkflowId: string }); update the two workflow 409
response entries to reference that new schema instead of ErrorResponse, then
regenerate the OpenAPI artifacts (do NOT hand-edit openapi/openapi.json or
src/types/openapi.ts) by running the repo's OpenAPI generate command so the
changes propagate into src/types/openapi.ts and the generated openapi JSON.
- Around line 9890-9896: The slug path parameter currently allows any string;
update the OpenAPI source that defines the parameter (so the generated symbol
for "slug" path param in the API spec) to include the server-side constraints
(pattern, minLength, maxLength) that match the server slug validation, then
re-run the OpenAPI generate command to regenerate openapi/openapi.json (do not
hand-edit the generated openapi.json). Ensure the updated source/type that
defines the parameter (the slug parameter definition used by the generator) is
changed so both places mentioned in the review (the slug path params around the
shown diff and the similar instance at lines ~9952-9958) get the pattern and
length bounds after regeneration.
In `@apps/api/src/routes/v1/workspaces/workflows.ts`:
- Around line 190-193: The current catch always calls
throwOnSlugConflict(workspaceId, req.body.slug ?? "", error) even when no slug
is being updated, causing unrelated unique-constraint 23505 errors to be
misclassified; update the promise chain around takeFirstOrNull so the catch only
routes errors through throwOnSlugConflict when req.body.slug != null/undefined
(i.e., only when a slug write is attempted), and additionally harden
throwOnSlugConflict to inspect the DB error.constraint (or equivalent) to ensure
the violation is the slug-unique-index before converting to DUPLICATE_SLUG
(otherwise rethrow the original error).
- Around line 35-42: The derived slug returned by slugifyWorkflowName should be
validated against slugSchema inside resolveSlug: after computing derived, run
slugSchema.safeParse (or parse) on derived and, if validation fails, throw an
ApiError with 400 and "INVALID_SLUG" including a helpful message; ensure you
import/Reference slugSchema and keep the existing empty-string check (or replace
it with the schema validation) so both explicit and derived slug paths are
validated consistently (locate resolveSlug, slugifyWorkflowName, and slugSchema
to implement this).
---
Nitpick comments:
In `@apps/api/openapi/paths/workflows.jsonnet`:
- Around line 64-81: Extract the duplicated inline requestBody schema into a
shared schema named CreateWorkflowRun (an object with required ['inputs'] and
inputs: {type: 'object', additionalProperties: true, description: 'Input values
for the workflow run.'}), add it to your central schemas collection (e.g.,
schemas.jsonnet), and replace the inline requestBody.schema in both the
slug-based endpoint and the ID-based endpoint with a reference to that shared
schema (e.g., $ref or the equivalent jsonnet reference to CreateWorkflowRun) so
both endpoints use a single source of truth.
In `@apps/api/src/routes/v1/workspaces/workflows.ts`:
- Around line 170-178: Extract the duplicate explicit-slug validation into a
single helper (e.g., validateSlug or ensureValidSlug) and use it from both the
inline check in this file and from resolveSlug so create and update paths share
the same logic; the helper should accept the candidate slug (the value currently
passed to slugSchema.safeParse) and, on validation failure, throw the same
ApiError with the message built from result.error.issues[0]?.message ?? "invalid
format" and code "INVALID_SLUG" so updateWorkflow and resolveSlug both call this
helper instead of duplicating the slugSchema.safeParse block.
In `@e2e/tests/api/workflows.spec.ts`:
- Around line 218-257: Update the duplicate-slug tests to assert the error
response body contains the conflicting slug and the existing workflow UUID:
after creating the first workflow (variables firstRes and firstId) and receiving
dupRes, assert dupRes.data (or dupRes.response.body) includes slug === the
generated slug and includes the existing workflow id (firstId) or a field like
conflictingWorkflowId; make the same assertions in the other duplicate-slug test
mentioned (lines 258-312) so the contract verifies both the 409 status and the
error payload contains the conflicting slug and workflow UUID.
- Around line 171-365: Replace the imperative create/delete calls in the
workflow E2E tests (tests like "should get a workflow by slug", "should reject
creating a workflow with a duplicate slug in the same workspace", "should reject
updating a workflow to a duplicate slug in the same workspace", etc.) with YAML
entity fixtures: add corresponding .spec.yaml fixtures describing the workflows
and slugs, load them at test start via importEntitiesFromYaml (using the same
workspace context) and remove the api.POST/api.DELETE setup/teardown, and ensure
tests call cleanupImportedEntities in afterEach/afterAll to tear down; update
any tests that depend on dynamic slugs to reference the fixture entity names
instead of creating them via api.POST.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 77b8b7a6-9782-45ef-96a8-86cf90fedbcb
📒 Files selected for processing (7)
apps/api/openapi/openapi.jsonapps/api/openapi/paths/workflows.jsonnetapps/api/src/routes/v1/workspaces/workflows.tsapps/api/src/types/openapi.tsapps/web/app/api/openapi.tse2e/api/schema.tse2e/tests/api/workflows.spec.ts
Comment on lines
+9890
to
+9896
| "description": "Slug of the workflow", | ||
| "in": "path", | ||
| "name": "slug", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string" | ||
| } |
Contributor
There was a problem hiding this comment.
Add slug format constraints to slug path parameters.
The new slug path params are unbounded string values. This omits the documented allowed-character/length contract and weakens generated-client validation. Add pattern and length bounds to these parameter schemas to match server-side slug validation behavior.
As per coding guidelines, "Do not hand-edit generated OpenAPI output (src/types/openapi.ts and openapi/openapi.json). Regenerate using the generate command instead."
Also applies to: 9952-9958
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/openapi/openapi.json` around lines 9890 - 9896, The slug path
parameter currently allows any string; update the OpenAPI source that defines
the parameter (so the generated symbol for "slug" path param in the API spec) to
include the server-side constraints (pattern, minLength, maxLength) that match
the server slug validation, then re-run the OpenAPI generate command to
regenerate openapi/openapi.json (do not hand-edit the generated openapi.json).
Ensure the updated source/type that defines the parameter (the slug parameter
definition used by the generator) is changed so both places mentioned in the
review (the slug path params around the shown diff and the similar instance at
lines ~9952-9958) get the pattern and length bounds after regeneration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #1131
Summary by CodeRabbit
New Features
Documentation
Tests