feat: Implement custom per-job pipeline stages - #244
Open
JoachimLK wants to merge 1 commit into
Open
Conversation
Replaced hard-coded status enums with a configurable `pipeline_stage` system. Jobs now define their own pipeline, allowing users to rename, recolor, reorder, and add stages. Automation rules now target these dynamic stages rather than a fixed action catalog. This change includes: - A new `pipeline_stage` table with category-based role definitions. - Migration to backfill existing applications and rules into this system. - A new stage management UI in the job settings. - Updated application and tracking APIs to use stage-based lookups.
JoachimLK
changed the title
📝 WalkthroughWalkthroughThis PR replaces fixed application statuses with customizable, job-specific pipeline stages. It adds stage management APIs and UI, migrates application and automation data to stage IDs, updates transitions and reporting, and introduces shared stage metadata, colors, defaults, validation, and tests. ChangesCustom pipeline stages
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StagePage
participant StageAPI
participant Database
User->>StagePage: Create or edit pipeline stage
StagePage->>StageAPI: POST/PATCH stage
StageAPI->>Database: Validate and persist stage
Database-->>StageAPI: Updated stage
StageAPI-->>StagePage: Stage response
StagePage->>StageAPI: Refresh job stages
StageAPI-->>StagePage: Ordered stage list
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
🚅 Deployed to the reqcore-pr-244 environment in applirank
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/rules/applyRules.ts (1)
28-38: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winConcurrency guard and audit trail both weakened by not tracking the prior
statusId.Both write paths guard only on
statusCategory === 'applied'and never capture the application's actual priorstatusId. Two consequences:
- The audit metadata is incomplete/inconsistent: the single-app path hardcodes
fromStageId: null, while the batch path omits the field entirely.- The optimistic-concurrency check only detects a category change, not a change of exact stage. If a job could ever have more than one
applied-category stage, a manual move between them between read and write would be silently overwritten.Fetching
statusIdup front and guarding on it instead of (or in addition to)statusCategoryis strictly more precise and fixes both issues at once.Suggested fix (single-application path)
const app = await db.query.application.findFirst({ where: and( eq(application.id, applicationId), eq(application.organizationId, organizationId), ), - columns: { id: true, jobId: true, statusCategory: true }, + columns: { id: true, jobId: true, statusId: true, statusCategory: true }, with: { stage: { columns: { name: true } } }, }) ... .where(and( eq(application.id, applicationId), eq(application.organizationId, organizationId), - // Guard against a concurrent manual change between read and write. - eq(application.statusCategory, 'applied'), + // Guard against a concurrent manual change between read and write — + // compare the exact prior stage, not just its category. + eq(application.statusId, app.statusId), )) ... metadata: { from: app.stage.name, to: targetName, - fromStageId: null, + fromStageId: app.statusId, toStageId: match.action, ruleId: match.ruleId, ruleName: match.ruleName, },Suggested fix (batch path)
const newApps = await db - .select({ id: application.id, statusName: pipelineStage.name }) + .select({ id: application.id, statusId: application.statusId, statusName: pipelineStage.name }) .from(application) ... - const toWrite: Array<{ id: string, fromName: string, match: RuleMatch }> = [] + const toWrite: Array<{ id: string, fromStatusId: string, fromName: string, match: RuleMatch }> = [] for (const app of newApps) { ... - toWrite.push({ id: app.id, fromName: app.statusName, match }) + toWrite.push({ id: app.id, fromStatusId: app.statusId, fromName: app.statusName, match }) } ... - await Promise.all(chunk.map(async ({ id, fromName, match }) => { + await Promise.all(chunk.map(async ({ id, fromStatusId, fromName, match }) => { const updated = await db.update(application) .set({ statusId: match.action, statusCategory: match.actionCategory, autoRule: match, updatedAt: new Date() }) .where(and( eq(application.id, id), eq(application.organizationId, organizationId), - eq(application.statusCategory, 'applied'), + eq(application.statusId, fromStatusId), ))Also applies to: 106-118, 142-149, 244-253, 299-313, 336-342
🤖 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 `@server/utils/rules/applyRules.ts` around lines 28 - 38, Update the application lookup in the single- and batch-processing paths to select and retain the current statusId alongside statusCategory. Use that exact statusId in the optimistic-concurrency guards so only applications remaining in the originally read applied stage are updated, and populate audit metadata with the captured prior statusId instead of null or omitting fromStageId; preserve the existing applied-category eligibility check as needed.
🧹 Nitpick comments (1)
server/api/interviews/index.post.ts (1)
67-72: 🎯 Functional Correctness | 🔵 TrivialAuto-advance-on-interview-scheduled behavior removed — confirm this is acceptable UX regression.
Recruiters previously had applications auto-move to an "interview" status when scheduling; now nothing moves the application automatically. The comment proposes a per-job opt-in setting to restore this. Happy to help scope/implement that follow-up (new stage flag + wiring here) if useful.
🤖 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 `@server/api/interviews/index.post.ts` around lines 67 - 72, Restore interview-scheduling auto-advance as an opt-in per-job setting: add a job-level target-stage configuration and, in the interview scheduling flow described by the surrounding code, move the application to that stage only when configured. Leave applications unchanged when the setting is absent, and use the selected job’s pipeline stage rather than assuming a fixed “interview” status.
🤖 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 `@app/components/ApplicationRulesBuilder.vue`:
- Around line 225-240: The validationError logic must block saving rules whose
target stage no longer exists; update validationError to call isMissingStage for
each rule and return an inline message identifying the rule and prompting
selection of another stage. Keep the existing validation checks and handleSave
flow unchanged for valid targets.
In `@app/components/CandidateDetailSidebar.vue`:
- Line 87: Move the stageColorClasses import to the top import block in
CandidateDetailSidebar.vue, alongside the existing imports, and remove its
current mid-file declaration without changing how it is used.
In `@app/pages/dashboard/index.vue`:
- Around line 62-76: The “To Review” card link still uses the legacy status=new
filter; update its navigation target to use the applications API’s
category-compatible filter for the applied stage. Locate the card route and
replace only the outdated status parameter, preserving the existing route and
other query parameters.
In `@app/pages/dashboard/jobs/`[id]/index.vue:
- Around line 367-375: Update describeTimelineItem to read the activity metadata
keys from and to before falling back to the legacy status keys, so
applyRulesToApplication stage transitions display their moved stages while
preserving existing legacy entries.
In `@app/pages/dashboard/jobs/index.vue`:
- Around line 181-183: Update totalActive to sum only pipeline.applied and
pipeline.in_progress, removing pipeline.hired from the Active count and sort
value while preserving the existing null-safe defaults.
In `@server/api/jobs/`[id]/stages/[stageId].delete.ts:
- Around line 49-84: Make the application-count validation and pipelineStage
deletion in the stage-delete handler race-safe: serialize concurrent stage moves
and deletion using the relevant row lock, or catch a resulting foreign-key
constraint failure and convert it to the same 422 reassignment-required response
used by applicationsInStage. Preserve the existing stage-count validation and
user-facing application message, and ensure no FK race produces an unhandled
500.
In `@server/database/migrations/0054_custom_pipeline_stages.sql`:
- Around line 61-62: Update
server/database/migrations/0054_custom_pipeline_stages.sql lines 61-62 to
enforce job-scoped ownership: add a unique (id, job_id) key for pipeline_stage
if needed, and make the application/status_id and
application_rule/target_stage_id foreign keys reference the corresponding
composite stage key with each row’s job_id. Update shared/status-transitions.ts
lines 12-15 only as needed to keep the documented guarantee aligned with the
database-enforced constraint.
In `@server/database/schema/app.ts`:
- Around line 217-232: Add a partial unique index in pipelineStage allowing at
most one isEntry row per jobId. In server/database/schema/app.ts lines 217-232,
define the index with the existing table callback and SQL predicate. In
server/api/jobs/[id]/stages/index.post.ts lines 27-61, protect the
check-and-insert flow with a transaction/lock or handle the new unique-violation
by retrying or returning a clear error. In
server/api/jobs/[id]/stages/[stageId].patch.ts lines 41-86, catch
unique-constraint violations from concurrent entry-stage changes and return HTTP
409 instead of leaving the transaction error unhandled.
In `@server/scripts/seed.ts`:
- Around line 9560-9571: Update the seeded status_changed activity generation
near the application insertion flow to resolve each transition through
stageByJobStatus using the relevant job and status, rather than emitting legacy
status strings. Populate the activity metadata with the resolved stage ID and
stage-based values so seeded timelines match production stage-change events.
In `@server/utils/candidate-import-service.ts`:
- Around line 283-285: Update the candidate import flow around entryStage and
linkToJob so a missing entry stage for targetJobId surfaces an error instead of
silently skipping job linkage. Match the existing failure behavior used by the
application creation endpoints, and ensure candidates are not reported as
successfully imported or linked when this prerequisite is absent.
---
Outside diff comments:
In `@server/utils/rules/applyRules.ts`:
- Around line 28-38: Update the application lookup in the single- and
batch-processing paths to select and retain the current statusId alongside
statusCategory. Use that exact statusId in the optimistic-concurrency guards so
only applications remaining in the originally read applied stage are updated,
and populate audit metadata with the captured prior statusId instead of null or
omitting fromStageId; preserve the existing applied-category eligibility check
as needed.
---
Nitpick comments:
In `@server/api/interviews/index.post.ts`:
- Around line 67-72: Restore interview-scheduling auto-advance as an opt-in
per-job setting: add a job-level target-stage configuration and, in the
interview scheduling flow described by the surrounding code, move the
application to that stage only when configured. Leave applications unchanged
when the setting is absent, and use the selected job’s pipeline stage rather
than assuming a fixed “interview” status.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a324ea4-b8d4-4938-83b7-d2d50180c6c8
📒 Files selected for processing (54)
app/components/AppTopBar.vueapp/components/ApplicationDetail.vueapp/components/ApplicationRulesBuilder.vueapp/components/ApplicationsList.vueapp/components/CandidateDetailDrawer.vueapp/components/CandidateDetailSidebar.vueapp/components/JobPipelineMini.vueapp/components/PipelineCard.vueapp/composables/useApplication.tsapp/composables/useApplications.tsapp/composables/useDashboard.tsapp/composables/useJobStages.tsapp/pages/dashboard/candidates/[id].vueapp/pages/dashboard/index.vueapp/pages/dashboard/jobs/[id]/index.vueapp/pages/dashboard/jobs/[id]/rules.vueapp/pages/dashboard/jobs/[id]/stages.vueapp/pages/dashboard/jobs/index.vueee/server/api/source-tracking/stats.get.tsserver/api/applications/[id].get.tsserver/api/applications/[id].patch.tsserver/api/applications/index.get.tsserver/api/applications/index.post.tsserver/api/candidates/[id].get.tsserver/api/dashboard/stats.get.tsserver/api/interviews/index.post.tsserver/api/jobs/[id]/rules/index.put.tsserver/api/jobs/[id]/stages/[stageId].delete.tsserver/api/jobs/[id]/stages/[stageId].patch.tsserver/api/jobs/[id]/stages/index.get.tsserver/api/jobs/[id]/stages/index.post.tsserver/api/jobs/[id]/stages/reorder.put.tsserver/api/jobs/index.get.tsserver/api/jobs/index.post.tsserver/api/public/jobs/[slug]/apply.post.tsserver/api/tracking-links/[id]/stats.get.tsserver/database/migrations/0054_custom_pipeline_stages.sqlserver/database/migrations/meta/0054_snapshot.jsonserver/database/migrations/meta/_journal.jsonserver/database/schema/app.tsserver/scripts/seed.tsserver/utils/ai/chatTools.tsserver/utils/candidate-import-service.tsserver/utils/pipeline.tsserver/utils/rules/applyRules.tsserver/utils/schemas/application.tsserver/utils/schemas/applicationRule.tsserver/utils/schemas/pipeline.tsshared/application-rules.tsshared/candidates.tsshared/pipeline.tsshared/status-transitions.tstests/unit/application-rules.test.tstests/unit/pipeline.test.ts
Comment on lines
+225
to
240
| // Targets come from the job's own pipeline — any stage is a valid destination. | ||
| const actionOptions = computed(() => | ||
| props.stages.map(s => ({ value: s.id, label: `Move to ${s.name}` })), | ||
| ) | ||
|
|
||
| /** Colour the arrow with the target stage's own colour. */ | ||
| function stageArrowClass(stageId: string): string { | ||
| const stage = props.stages.find(s => s.id === stageId) | ||
| return stageColorClasses(stage?.color).badge | ||
| } | ||
|
|
||
| const actionBadgeClass: Record<RuleAction, string> = { | ||
| rejected: 'text-danger-600 dark:text-danger-400', | ||
| screening: 'text-warning-600 dark:text-warning-400', | ||
| interview: 'text-brand-600 dark:text-brand-400', | ||
| /** A rule whose target stage was deleted elsewhere — surfaced for cleanup. */ | ||
| function isMissingStage(stageId: string): boolean { | ||
| return !!stageId && !props.stages.some(s => s.id === stageId) | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing target stage isn't blocked from saving.
isMissingStage only drives a visual warning; validationError (unchanged, above) never checks it, so handleSave will still emit a rule pointing at a deleted stage. This bulk-save is atomic server-side (index.put.ts rejects the whole batch with a 422), so the user gets a generic "Failed to save rules" toast instead of the specific inline guidance they're already seeing.
Add a check to validationError, e.g.:
if (isMissingStage(r.targetStageId)) return `"${r.name}" targets a deleted stage — pick another.`Also applies to: 452-466
🤖 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 `@app/components/ApplicationRulesBuilder.vue` around lines 225 - 240, The
validationError logic must block saving rules whose target stage no longer
exists; update validationError to call isMissingStage for each rule and return
an inline message identifying the rule and prompting selection of another stage.
Keep the existing validation checks and handleSave flow unchanged for valid
targets.
| hired: 'Hired', | ||
| rejected: 'Reject', | ||
| } | ||
| import { stageColorClasses } from '~~/shared/pipeline' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the stageColorClasses import to the top of the script.
It's declared mid-file (after other refs/computeds) instead of with the other imports at lines 2-8. Works due to import hoisting, but breaks the file's import grouping and will likely trip an import/first-style lint rule.
♻️ Proposed fix
import type { CandidateDetail } from '~~/shared/candidates'
import { usePreviewReadOnly } from '~/composables/usePreviewReadOnly'
+import { stageColorClasses } from '~~/shared/pipeline'-import { stageColorClasses } from '~~/shared/pipeline'
-
// Stage moves are free-form across the job's custom pipeline: every stage except
// the one the application is currently in.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { stageColorClasses } from '~~/shared/pipeline' |
🤖 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 `@app/components/CandidateDetailSidebar.vue` at line 87, Move the
stageColorClasses import to the top import block in CandidateDetailSidebar.vue,
alongside the existing imports, and remove its current mid-file declaration
without changing how it is used.
Comment on lines
+62
to
76
| // Cross-job dashboard aggregates by stage category (each job has its own custom | ||
| // stages, so the funnel is shown by the role a stage plays). | ||
| const stageConfig = [ | ||
| { key: 'new', label: 'New', color: 'bg-blue-500', textColor: 'text-blue-600 dark:text-blue-400', bgColor: 'bg-blue-50 dark:bg-blue-950/40' }, | ||
| { key: 'screening', label: 'Screening', color: 'bg-violet-500', textColor: 'text-violet-600 dark:text-violet-400', bgColor: 'bg-violet-50 dark:bg-violet-950/40' }, | ||
| { key: 'interview', label: 'Interview', color: 'bg-amber-500', textColor: 'text-amber-600 dark:text-amber-400', bgColor: 'bg-amber-50 dark:bg-amber-950/40' }, | ||
| { key: 'offer', label: 'Offer', color: 'bg-teal-500', textColor: 'text-teal-600 dark:text-teal-400', bgColor: 'bg-teal-50 dark:bg-teal-950/40' }, | ||
| { key: 'applied', label: 'Applied', color: 'bg-blue-500', textColor: 'text-blue-600 dark:text-blue-400', bgColor: 'bg-blue-50 dark:bg-blue-950/40' }, | ||
| { key: 'in_progress', label: 'In progress', color: 'bg-amber-500', textColor: 'text-amber-600 dark:text-amber-400', bgColor: 'bg-amber-50 dark:bg-amber-950/40' }, | ||
| { key: 'hired', label: 'Hired', color: 'bg-green-600', textColor: 'text-green-600 dark:text-green-400', bgColor: 'bg-green-50 dark:bg-green-950/40' }, | ||
| { key: 'rejected', label: 'Rejected', color: 'bg-surface-400', textColor: 'text-surface-500 dark:text-surface-400', bgColor: 'bg-surface-100 dark:bg-surface-800' }, | ||
| ] as const | ||
|
|
||
| const stageCountKeys: Record<string, string> = { | ||
| new: 'newCount', | ||
| screening: 'screeningCount', | ||
| interview: 'interviewCount', | ||
| offer: 'offerCount', | ||
| applied: 'appliedCount', | ||
| in_progress: 'inProgressCount', | ||
| hired: 'hiredCount', | ||
| rejected: 'rejectedCount', | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Update the “To Review” route to use the stage-based filter.
The page now aggregates entry-stage applications as applied, but the existing card link still sends status=new. That legacy value cannot identify a dynamic stage after this migration, so the card can open an empty or unfiltered list. Route with the applications API’s category-compatible filter instead.
🤖 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 `@app/pages/dashboard/index.vue` around lines 62 - 76, The “To Review” card
link still uses the legacy status=new filter; update its navigation target to
use the applications API’s category-compatible filter for the applied stage.
Locate the card route and replace only the outdated status parameter, preserving
the existing route and other query parameters.
Comment on lines
+367
to
+375
| /** | ||
| * Timeline entries record stage NAMES (stages are renameable, so the historical | ||
| * name is what was true at the time). Colour by the current stage of that name | ||
| * when it still exists, else a neutral badge. | ||
| */ | ||
| function getTimelineStatusBadge(status: string): string { | ||
| const s = status.toLowerCase() | ||
| const map: Record<string, string> = { | ||
| new: 'bg-blue-100 text-blue-700 dark:bg-blue-900/60 dark:text-blue-300', | ||
| screening: 'bg-violet-100 text-violet-700 dark:bg-violet-900/60 dark:text-violet-300', | ||
| interview: 'bg-amber-100 text-amber-700 dark:bg-amber-900/60 dark:text-amber-300', | ||
| offer: 'bg-teal-100 text-teal-700 dark:bg-teal-900/60 dark:text-teal-300', | ||
| hired: 'bg-green-100 text-green-700 dark:bg-green-900/60 dark:text-green-300', | ||
| rejected: 'bg-surface-200 text-surface-600 dark:bg-surface-700 dark:text-surface-300', | ||
| } | ||
| return map[s] ?? 'bg-surface-100 text-surface-600 dark:bg-surface-800 dark:text-surface-300' | ||
| const match = stages.value.find(s => s.name.toLowerCase() === status.toLowerCase()) | ||
| if (match) return stageColorClasses(match.color).badge | ||
| return 'bg-surface-100 text-surface-600 dark:bg-surface-800 dark:text-surface-300' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Read the new activity metadata keys.
applyRulesToApplication records stage changes as from and to, while describeTimelineItem only reads the legacy status keys. Auto-rule transitions therefore render as a generic activity entry instead of showing the moved stages. Accept from/to first, then retain the legacy fallbacks.
🤖 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 `@app/pages/dashboard/jobs/`[id]/index.vue around lines 367 - 375, Update
describeTimelineItem to read the activity metadata keys from and to before
falling back to the legacy status keys, so applyRulesToApplication stage
transitions display their moved stages while preserving existing legacy entries.
Comment on lines
181
to
183
| function totalActive(pipeline: any) { | ||
| return (pipeline?.new ?? 0) + (pipeline?.screening ?? 0) + (pipeline?.interview ?? 0) + (pipeline?.offer ?? 0) + (pipeline?.hired ?? 0) | ||
| return (pipeline?.applied ?? 0) + (pipeline?.in_progress ?? 0) + (pipeline?.hired ?? 0) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude hired candidates from “Active”.
hired is terminal in STAGE_CATEGORY_META, so including it inflates the Active column and its sort order. Count only applied and in_progress.
🤖 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 `@app/pages/dashboard/jobs/index.vue` around lines 181 - 183, Update
totalActive to sum only pipeline.applied and pipeline.in_progress, removing
pipeline.hired from the Active count and sort value while preserving the
existing null-safe defaults.
Comment on lines
+49
to
+84
| const [totalStages, [appCount]] = await Promise.all([ | ||
| db.$count(pipelineStage, and( | ||
| eq(pipelineStage.jobId, jobId), | ||
| eq(pipelineStage.organizationId, orgId), | ||
| )), | ||
| db.select({ count: count() }) | ||
| .from(application) | ||
| .where(and( | ||
| eq(application.statusId, stageId), | ||
| eq(application.organizationId, orgId), | ||
| )), | ||
| ]) | ||
|
|
||
| if (totalStages <= 1) { | ||
| throw createError({ statusCode: 422, statusMessage: 'A job must keep at least one stage' }) | ||
| } | ||
|
|
||
| const applicationsInStage = Number(appCount?.count ?? 0) | ||
| if (applicationsInStage > 0) { | ||
| throw createError({ | ||
| statusCode: 422, | ||
| statusMessage: `${applicationsInStage} application${applicationsInStage === 1 ? '' : 's'} still in "${stage.name}". Move them to another stage first.`, | ||
| }) | ||
| } | ||
|
|
||
| const removedRules = await db.$count(applicationRule, and( | ||
| eq(applicationRule.targetStageId, stageId), | ||
| eq(applicationRule.organizationId, orgId), | ||
| )) | ||
|
|
||
| await db.delete(pipelineStage) | ||
| .where(and( | ||
| eq(pipelineStage.id, stageId), | ||
| eq(pipelineStage.organizationId, orgId), | ||
| )) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the application check and deletion race-safe.
A concurrent stage move can occur after appCount is read and before the delete. The FK then rejects line 79 with an unhandled constraint error, producing a 500 instead of the intended 422. Serialize this operation with the relevant row lock, or translate the FK violation into the same reassignment-required response.
🤖 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 `@server/api/jobs/`[id]/stages/[stageId].delete.ts around lines 49 - 84, Make
the application-count validation and pipelineStage deletion in the stage-delete
handler race-safe: serialize concurrent stage moves and deletion using the
relevant row lock, or catch a resulting foreign-key constraint failure and
convert it to the same 422 reassignment-required response used by
applicationsInStage. Preserve the existing stage-count validation and
user-facing application message, and ensure no FK race produces an unhandled
500.
Comment on lines
+61
to
+62
| ALTER TABLE "application" ADD CONSTRAINT "application_status_id_pipeline_stage_id_fk" FOREIGN KEY ("status_id") REFERENCES "public"."pipeline_stage"("id") ON DELETE restrict ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "application_rule" ADD CONSTRAINT "application_rule_target_stage_id_pipeline_stage_id_fk" FOREIGN KEY ("target_stage_id") REFERENCES "public"."pipeline_stage"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Enforce stage ownership in the database. These foreign keys validate only that a stage exists; they permit an application or rule for Job A to reference a stage from Job B. This violates the documented job-scoped pipeline contract.
server/database/migrations/0054_custom_pipeline_stages.sql#L61-L62: Add composite ownership constraints, such as(status_id, job_id)and(target_stage_id, job_id)referencing a unique(id, job_id)stage key.shared/status-transitions.ts#L12-L15: Keep this guarantee documented only once the database or all mutation boundaries enforce it.
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 61-61: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 61-61: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 62-62: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 62-62: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
📍 Affects 2 files
server/database/migrations/0054_custom_pipeline_stages.sql#L61-L62(this comment)shared/status-transitions.ts#L12-L15
🤖 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 `@server/database/migrations/0054_custom_pipeline_stages.sql` around lines 61 -
62, Update server/database/migrations/0054_custom_pipeline_stages.sql lines
61-62 to enforce job-scoped ownership: add a unique (id, job_id) key for
pipeline_stage if needed, and make the application/status_id and
application_rule/target_stage_id foreign keys reference the corresponding
composite stage key with each row’s job_id. Update shared/status-transitions.ts
lines 12-15 only as needed to keep the documented guarantee aligned with the
database-enforced constraint.
Comment on lines
+217
to
+232
| export const pipelineStage = pgTable('pipeline_stage', { | ||
| id: text('id').primaryKey().$defaultFn(() => crypto.randomUUID()), | ||
| organizationId: text('organization_id').notNull().references(() => organization.id, { onDelete: 'cascade' }), | ||
| jobId: text('job_id').notNull().references(() => job.id, { onDelete: 'cascade' }), | ||
| name: text('name').notNull(), | ||
| color: text('color').notNull().default('slate'), | ||
| category: stageCategoryEnum('category').notNull().default('in_progress'), | ||
| displayOrder: integer('display_order').notNull().default(0), | ||
| /** Exactly one stage per job is the entry point for new applications. */ | ||
| isEntry: boolean('is_entry').notNull().default(false), | ||
| createdAt: timestamp('created_at').notNull().defaultNow(), | ||
| updatedAt: timestamp('updated_at').notNull().defaultNow(), | ||
| }, (t) => ([ | ||
| index('pipeline_stage_organization_id_idx').on(t.organizationId), | ||
| index('pipeline_stage_job_id_idx').on(t.jobId), | ||
| ])) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Entry-stage invariant is only enforced in application code, not the database — three sites can race into an inconsistent state.
pipelineStage allows multiple isEntry = true rows per jobId at the DB level; the two API handlers that maintain this invariant do so via unlocked read-then-write sequences, so concurrent requests can violate it.
server/database/schema/app.ts#L217-L232: add a partial unique index (e.g.uniqueIndex('pipeline_stage_one_entry_per_job_idx').on(t.jobId).where(sql\${t.isEntry} = true`)`) so the invariant holds regardless of application-layer races.server/api/jobs/[id]/stages/index.post.ts#L27-L61: two concurrentPOSTrequests on an empty pipeline can both observeexisting.length === 0and both insertisEntry: true; wrap the check+insert in a transaction/lock, or catch the unique-violation from the new index and retry/return a clear error.server/api/jobs/[id]/stages/[stageId].patch.ts#L41-L86: two concurrent PATCH requests movingisEntryto different stages can interleave (or deadlock on the overlapping row updates); once the constraint exists, catch the violation and surface a 409 instead of letting Postgres abort the transaction unhandled.
📍 Affects 3 files
server/database/schema/app.ts#L217-L232(this comment)server/api/jobs/[id]/stages/index.post.ts#L27-L61server/api/jobs/[id]/stages/[stageId].patch.ts#L41-L86
🤖 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 `@server/database/schema/app.ts` around lines 217 - 232, Add a partial unique
index in pipelineStage allowing at most one isEntry row per jobId. In
server/database/schema/app.ts lines 217-232, define the index with the existing
table callback and SQL predicate. In server/api/jobs/[id]/stages/index.post.ts
lines 27-61, protect the check-and-insert flow with a transaction/lock or handle
the new unique-violation by retrying or returning a clear error. In
server/api/jobs/[id]/stages/[stageId].patch.ts lines 41-86, catch
unique-constraint violations from concurrent entry-stage changes and return HTTP
409 instead of leaving the transaction error unhandled.
Comment on lines
+9560
to
+9571
| const stage = stageByJobStatus.get(`${jobId}:${app.status}`); | ||
| if (!stage) { | ||
| throw new Error(`Missing seeded stage for job ${jobId} status ${app.status}`); | ||
| } | ||
|
|
||
| await db.insert(schema.application).values({ | ||
| id: applicationId, | ||
| organizationId: orgId, | ||
| candidateId, | ||
| jobId, | ||
| status: app.status, | ||
| statusId: stage.id, | ||
| statusCategory: stage.category, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep seeded activity metadata stage-based.
Applications now resolve real stage IDs, but the later seeded status_changed activities still emit legacy new/screening/... strings without stage IDs. Resolve those transitions through stageByJobStatus too, so demo timelines match production stage-change events.
🤖 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 `@server/scripts/seed.ts` around lines 9560 - 9571, Update the seeded
status_changed activity generation near the application insertion flow to
resolve each transition through stageByJobStatus using the relevant job and
status, rather than emitting legacy status strings. Populate the activity
metadata with the resolved stage ID and stage-based values so seeded timelines
match production stage-change events.
Comment on lines
+283
to
+285
| // Resolve the target job's entry stage once; every imported applicant lands there. | ||
| const entryStage = targetJobId ? await getEntryStage(targetJobId, orgId) : null | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Silent no-op when the target job has no entry stage.
linkToJob silently returns when entryStage is null (line 317), while the identical precondition throws a 500 in server/api/applications/index.post.ts and server/api/public/jobs/[slug]/apply.post.ts. Here, candidates are still created/updated and counted as result.created/result.updated, but never linked to the target job — with no log or error indicating why. If the "every job always has stages" invariant is ever violated, users will believe the import succeeded and candidates were added to the pipeline when they weren't.
🛡️ Proposed fix: surface the failure instead of silently skipping
async function linkToJob(candidateId: string) {
- if (!targetJobId || !entryStage) return
+ if (!targetJobId) return
+ if (!entryStage) {
+ // Should be unreachable (every job is seeded with a default pipeline),
+ // but if it ever happens, don't silently pretend the link succeeded.
+ console.error(`commitImport: job ${targetJobId} has no pipeline stages; skipping application link for candidate ${candidateId}`)
+ return
+ }
const [app] = await db.insert(application)
.values({ organizationId: orgId, candidateId, jobId: targetJobId, statusId: entryStage.id, statusCategory: entryStage.category })Also applies to: 315-325
🤖 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 `@server/utils/candidate-import-service.ts` around lines 283 - 285, Update the
candidate import flow around entryStage and linkToJob so a missing entry stage
for targetJobId surfaces an error instead of silently skipping job linkage.
Match the existing failure behavior used by the application creation endpoints,
and ensure candidates are not reported as successfully imported or linked when
this prerequisite is absent.
This was referenced Jul 27, 2026
Merged
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
1 participant
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.
Replaced hard-coded status enums with a configurable
pipeline_stagesystem. Jobs now define their own pipeline, allowing users to rename, recolor, reorder, and add stages. Automation rules now target these dynamic stages rather than a fixed action catalog.This change includes:
pipeline_stagetable with category-based role definitions.Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit