Skip to content

Commit 7ed96b5

Browse files
committed
improvement(mothership): run the job lane unflagged behind one indexed probe
1 parent 07478f5 commit 7ed96b5

11 files changed

Lines changed: 152 additions & 45 deletions

File tree

‎apps/sim/.env.example‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ NEXT_PUBLIC_APP_URL=http://localhost:3000
2929
# MSHIP_SYSPROMPT_OVERRIDE= # Highest-priority instructions for Mothership; honored only when the validated API key owner is enterprise
3030
# NEXT_PUBLIC_CHAT_DISABLED=true # Hides the Chat module: the workspace lands on your first workflow, and the chats list, scheduled tasks, and editor Chat panel are absent. Chat is shown when unset; `bun run setup` sets this for you if you skip the chat key
3131
# MOTHERSHIP_BENCHMARK_ENABLED=true # Shows the organization Benchmark tab and enables its authenticated APIs
32-
# MOTHERSHIP_JOBS_ENABLED=true # Runs scheduled agent jobs from the schedule tick (unreleased; keep off in production)
3332
# COPILOT_DEV_URL=http://localhost:8080 # Benchmark worker endpoint; requires the worker benchmark profile and the normal COPILOT_API_KEY
3433

3534
# Remote Function sandboxes (Optional)

‎apps/sim/app/api/schedules/execute/route.test.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,9 @@ vi.mock('@/lib/execution/manual-cancellation', () => ({
108108

109109
vi.mock('@/lib/core/async-jobs', () => asyncJobsMock)
110110

111-
/** The job lane is off by default and has its own integration suite. */
111+
/** The job lane has its own integration suite. */
112112
vi.mock('@/lib/mothership/jobs/lane', () => ({
113-
isJobLaneEnabled: () => false,
114-
runJobLane: vi.fn(),
113+
runJobLane: vi.fn(async () => ({ claimed: 0 })),
115114
}))
116115

117116
vi.mock('drizzle-orm', () => ({

‎apps/sim/app/api/schedules/execute/route.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import {
5050
registerManualExecutionAborter,
5151
unregisterManualExecutionAborter,
5252
} from '@/lib/execution/manual-cancellation'
53-
import { isJobLaneEnabled, runJobLane } from '@/lib/mothership/jobs/lane'
53+
import { runJobLane } from '@/lib/mothership/jobs/lane'
5454
import {
5555
buildCarrierReconciledMetadata,
5656
carrierNotReconciledSql,
@@ -1789,13 +1789,11 @@ export async function runScheduleTick(requestId: string): Promise<ScheduleTickRe
17891789
await Promise.allSettled(schedulePromises)
17901790
}
17911791

1792-
if (isJobLaneEnabled()) {
1793-
try {
1794-
const { claimed } = await runJobLane()
1795-
totalSchedules += claimed
1796-
} catch (error) {
1797-
logger.error(`[${requestId}] Scheduled job lane failed`, { error: toError(error).message })
1798-
}
1792+
try {
1793+
const { claimed } = await runJobLane()
1794+
totalSchedules += claimed
1795+
} catch (error) {
1796+
logger.error(`[${requestId}] Scheduled job lane failed`, { error: toError(error).message })
17991797
}
18001798

18011799
const totalCount = totalSchedules

‎apps/sim/lib/core/config/env-flags.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,6 @@ export const isCopilotToolPermissionsEnabled = isTruthy(env.COPILOT_TOOL_PERMISS
111111
/** Deployment-only opt-in; authenticated resource permissions still apply. */
112112
export const isMothershipBenchmarkEnabled = isTruthy(env.MOTHERSHIP_BENCHMARK_ENABLED)
113113

114-
/**
115-
* Scheduled agent jobs: the schedule tick claims due job occurrences and runs each as an
116-
* unattended durable turn in its job's chat. Off by default, and must stay off in production
117-
* until unattended runs hold production effects for human approval: until then an unattended
118-
* run can do anything its owner can.
119-
*/
120-
export const isMothershipJobsEnabled = isTruthy(env.MOTHERSHIP_JOBS_ENABLED)
121-
122114
/**
123115
* Is billing enforcement enabled.
124116
*

‎apps/sim/lib/core/config/env.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,6 @@ export const env = createEnv({
151151
COPILOT_TOOL_PERMISSIONS_ENABLED: z.boolean().optional(),
152152
/** Enables the organization benchmark UI and its authenticated endpoints. */
153153
MOTHERSHIP_BENCHMARK_ENABLED: z.boolean().optional(),
154-
/** Runs scheduled agent jobs from the schedule tick. Off by default; keep off in production. */
155-
MOTHERSHIP_JOBS_ENABLED: z.boolean().optional(),
156154
MOTHERSHIP_BENCHMARK_URL: z.string().url().optional(),
157155
SIM_AGENT_API_URL: z.string().url().optional(), // URL for internal sim agent API
158156
MSHIP_SYSPROMPT_OVERRIDE: z.string().min(1).optional(), // Enterprise-only highest-priority Mothership system prompt override forwarded by Sim

‎apps/sim/lib/mothership/jobs/claim.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { jobOccurrenceId, nextJobOccurrence } from '@/lib/mothership/jobs/occurr
55
/** The trigger every job run record carries; the logs surface lists runs under it. */
66
export const JOB_RUN_TRIGGER = 'mothership'
77

8-
const dueJob = (now: Date) =>
8+
/** A live job whose next occurrence is due; `workflow_schedule_due_job_idx` covers it. */
9+
export const dueJob = (now: Date) =>
910
and(
1011
eq(workflowSchedule.sourceType, 'job'),
1112
isNull(workflowSchedule.archivedAt),

‎apps/sim/lib/mothership/jobs/lane.integration.ts‎

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,25 @@ const { redisUrl, inheritedEnv, worker } = await vi.hoisted(async () => {
8888
const inheritedEnv = {
8989
REDIS_URL: process.env.REDIS_URL,
9090
SIM_AGENT_API_URL: process.env.SIM_AGENT_API_URL,
91+
COPILOT_API_KEY: process.env.COPILOT_API_KEY,
9192
}
9293
if (url) process.env.REDIS_URL = url
94+
process.env.COPILOT_API_KEY = 'job-lane-fixture-key'
9395
process.env.SIM_AGENT_API_URL = `http://127.0.0.1:${port}`
9496
return { redisUrl: url, inheritedEnv, worker: Object.assign(worker, { server, finish }) }
9597
})
9698

9799
vi.mock('@/lib/core/config/env-flags', () => envFlagsMock)
100+
101+
/** Spied so the idle tick can prove it neither claims nor reconciles. */
102+
vi.mock('@/lib/mothership/jobs/claim', async (importOriginal) => {
103+
const actual = await importOriginal<typeof import('@/lib/mothership/jobs/claim')>()
104+
return { ...actual, claimDueJobOccurrences: vi.fn(actual.claimDueJobOccurrences) }
105+
})
106+
vi.mock('@/lib/mothership/jobs/reconcile', async (importOriginal) => {
107+
const actual = await importOriginal<typeof import('@/lib/mothership/jobs/reconcile')>()
108+
return { ...actual, reconcileJobRuns: vi.fn(actual.reconcileJobRuns) }
109+
})
98110
vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
99111

100112
const { controllers } = vi.hoisted(() => ({ controllers: new Map<string, AbortController>() }))
@@ -122,15 +134,15 @@ import {
122134
} from '@sim/db/schema'
123135
import { sleep } from '@sim/utils/helpers'
124136
import { generateId } from '@sim/utils/id'
125-
import { eq, sql } from 'drizzle-orm'
137+
import { eq, inArray, sql } from 'drizzle-orm'
126138
import { closeRedisConnection, getRedisClient } from '@/lib/core/config/redis'
127139
import { ABANDONED_RUN_CURSOR_KEY } from '@/lib/mothership/async-runs/orphaned-runs'
128140
import { chatPubSub } from '@/lib/mothership/chat-status'
129141
import { recordJobRunSpend } from '@/lib/mothership/jobs/budget'
130142
import { claimDueJobOccurrences } from '@/lib/mothership/jobs/claim'
131143
import { dispatchJobOccurrence } from '@/lib/mothership/jobs/dispatch'
132144
import { finalizeJobRun } from '@/lib/mothership/jobs/finalize'
133-
import { runJobLane } from '@/lib/mothership/jobs/lane'
145+
import { jobLaneWorkQuery, runJobLane } from '@/lib/mothership/jobs/lane'
134146
import { jobOccurrenceId, MIN_JOB_CADENCE_MS } from '@/lib/mothership/jobs/occurrence'
135147
import { JOB_DISPATCH_GRACE_MS, reconcileJobRuns } from '@/lib/mothership/jobs/reconcile'
136148
import { takeOverAbandonedRuns } from '@/lib/mothership/request/application/abandoned-runs'
@@ -140,6 +152,18 @@ import {
140152
StreamControllerSupersededError,
141153
} from '@/lib/mothership/request/session/controller-lease'
142154

155+
interface QueryPlan {
156+
'Node Type': string
157+
'Index Name'?: string
158+
'Shared Hit Blocks': number
159+
'Shared Read Blocks': number
160+
Plans?: QueryPlan[]
161+
}
162+
163+
function planNodes(plan: QueryPlan): QueryPlan[] {
164+
return [plan, ...(plan.Plans ?? []).flatMap(planNodes)]
165+
}
166+
143167
const userId = generateId()
144168
const workspaceId = generateId()
145169
const chatIds: string[] = []
@@ -316,6 +340,61 @@ describe.runIf(Boolean(redisUrl))('the scheduled job lane', () => {
316340
await closeRedisConnection()
317341
})
318342

343+
it('does nothing but one indexed probe on a tick with no live jobs', async () => {
344+
/** What the archive migration leaves behind: retired jobs and their finished run logs. */
345+
const archivedIds = Array.from({ length: 2_000 }, () => generateId())
346+
await db.insert(workflowSchedule).values(
347+
archivedIds.map((id) => ({
348+
id,
349+
triggerType: 'schedule',
350+
sourceType: 'job',
351+
status: 'disabled',
352+
archivedAt: new Date(),
353+
nextRunAt: new Date(Date.now() - 60_000),
354+
sourceUserId: userId,
355+
sourceWorkspaceId: workspaceId,
356+
}))
357+
)
358+
await db.insert(jobExecutionLogs).values(
359+
Array.from({ length: 5_000 }, () => {
360+
const id = generateId()
361+
return {
362+
id,
363+
executionId: id,
364+
workspaceId,
365+
level: 'info',
366+
status: 'completed',
367+
trigger: 'mothership',
368+
startedAt: new Date(Date.now() - 60_000),
369+
}
370+
})
371+
)
372+
await db.execute(sql`ANALYZE workflow_schedule, job_execution_logs`)
373+
const now = new Date()
374+
vi.mocked(claimDueJobOccurrences).mockClear()
375+
vi.mocked(reconcileJobRuns).mockClear()
376+
377+
try {
378+
expect(await runJobLane(now)).toEqual({ claimed: 0 })
379+
expect(claimDueJobOccurrences).not.toHaveBeenCalled()
380+
expect(reconcileJobRuns).not.toHaveBeenCalled()
381+
382+
const plans = await db.execute<{ 'QUERY PLAN': { Plan: QueryPlan }[] }>(
383+
sql`EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${jobLaneWorkQuery(now)}`
384+
)
385+
const plan = plans[0]['QUERY PLAN'][0].Plan
386+
const nodes = planNodes(plan)
387+
expect(nodes.some((node) => node['Node Type'] === 'Seq Scan')).toBe(false)
388+
expect(nodes.map((node) => node['Index Name']).filter(Boolean)).toEqual(
389+
expect.arrayContaining(['workflow_schedule_due_job_idx', 'job_execution_logs_open_idx'])
390+
)
391+
expect(plan['Shared Hit Blocks'] + plan['Shared Read Blocks']).toBeLessThan(20)
392+
} finally {
393+
await db.delete(jobExecutionLogs).where(eq(jobExecutionLogs.workspaceId, workspaceId))
394+
await db.delete(workflowSchedule).where(inArray(workflowSchedule.id, archivedIds))
395+
}
396+
})
397+
319398
it('records an occurrence once when two ticks claim it at the same time, and starts it once', async () => {
320399
const { scheduleId, occurrenceId, dueAt } = await seedJob()
321400
const now = new Date()
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { dbChainMockFns, resetDbChainMock } from '@sim/testing/mocks/database.mock'
5+
import { resetEnvMock, setEnv } from '@sim/testing/mocks/env.mock'
6+
import { afterEach, describe, expect, it } from 'vitest'
7+
import { runJobLane } from '@/lib/mothership/jobs/lane'
8+
9+
describe('runJobLane', () => {
10+
afterEach(() => {
11+
resetEnvMock()
12+
resetDbChainMock()
13+
})
14+
15+
it.each([
16+
{ COPILOT_API_KEY: undefined, REDIS_URL: 'redis://localhost:6379' },
17+
{ COPILOT_API_KEY: 'key', REDIS_URL: undefined },
18+
])('touches nothing where turns cannot be durable (%o)', async (env) => {
19+
setEnv(env)
20+
21+
expect(await runJobLane()).toEqual({ claimed: 0 })
22+
expect(dbChainMockFns.execute).not.toHaveBeenCalled()
23+
expect(dbChainMockFns.transaction).not.toHaveBeenCalled()
24+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
25+
})
26+
})
Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,43 @@
1+
import { db, jobExecutionLogs, workflowSchedule } from '@sim/db'
12
import { createLogger } from '@sim/logger'
23
import { getErrorMessage } from '@sim/utils/errors'
4+
import { exists, sql } from 'drizzle-orm'
35
import { env } from '@/lib/core/config/env'
4-
import { isMothershipJobsEnabled } from '@/lib/core/config/env-flags'
5-
import { claimDueJobOccurrences } from '@/lib/mothership/jobs/claim'
6+
import { claimDueJobOccurrences, dueJob } from '@/lib/mothership/jobs/claim'
67
import { dispatchJobOccurrence } from '@/lib/mothership/jobs/dispatch'
7-
import { reconcileJobRuns } from '@/lib/mothership/jobs/reconcile'
8+
import { openJobRun, reconcileJobRuns } from '@/lib/mothership/jobs/reconcile'
89

910
const logger = createLogger('MothershipJobLane')
1011

1112
/** Occurrences one tick claims; the rest wait for the next tick. */
1213
const JOB_CLAIM_LIMIT = 50
1314

1415
/**
15-
* Jobs run only where their turns are durable: the flag is on, the worker is configured, and
16-
* Redis holds the chat locks that let Sim take a turn over after its process dies.
16+
* One row: `due`, whether a job's occurrence is due to be claimed, and `open`, whether an
17+
* occurrence is still queued or running. Each leg probes its own partial index, so a deployment
18+
* with no live jobs pays one index lookup per tick and nothing else.
1719
*/
18-
export function isJobLaneEnabled(): boolean {
19-
return isMothershipJobsEnabled && Boolean(env.COPILOT_API_KEY) && Boolean(env.REDIS_URL)
20+
export function jobLaneWorkQuery(now: Date) {
21+
const due = db.select({ id: workflowSchedule.id }).from(workflowSchedule).where(dueJob(now))
22+
const open = db
23+
.select({ id: jobExecutionLogs.id })
24+
.from(jobExecutionLogs)
25+
.where(openJobRun(['queued', 'running']))
26+
return sql`SELECT ${exists(due.limit(1))} AS due, ${exists(open.limit(1))} AS open`
2027
}
2128

2229
/**
2330
* The schedule tick's job lane: claims due occurrences, starts each as a durable turn, then
2431
* drives every open occurrence (including ones a crashed tick left) towards its end.
32+
*
33+
* Jobs run only where their turns are durable: the worker is configured, and Redis holds the
34+
* chat locks that let Sim take a turn over after its process dies. Elsewhere the lane does
35+
* nothing, not even a query.
2536
*/
2637
export async function runJobLane(now: Date = new Date()): Promise<{ claimed: number }> {
27-
const claimed = await claimDueJobOccurrences(now, JOB_CLAIM_LIMIT)
38+
if (!env.COPILOT_API_KEY || !env.REDIS_URL) return { claimed: 0 }
39+
const [work] = await db.execute<{ due: boolean; open: boolean }>(jobLaneWorkQuery(now))
40+
const claimed = work?.due ? await claimDueJobOccurrences(now, JOB_CLAIM_LIMIT) : []
2841
for (const occurrence of claimed) {
2942
await dispatchJobOccurrence(occurrence.executionId).catch((error) =>
3043
logger.warn('Job occurrence dispatch failed; the reconciler retries it', {
@@ -33,6 +46,6 @@ export async function runJobLane(now: Date = new Date()): Promise<{ claimed: num
3346
})
3447
)
3548
}
36-
await reconcileJobRuns(now)
49+
if (work?.open || claimed.length > 0) await reconcileJobRuns(now)
3750
return { claimed: claimed.length }
3851
}

‎apps/sim/lib/mothership/jobs/reconcile.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { copilotRuns, db, jobExecutionLogs, workflowSchedule } from '@sim/db'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
4-
import { and, asc, eq, isNotNull, sql } from 'drizzle-orm'
4+
import { and, asc, eq, inArray, isNotNull, sql } from 'drizzle-orm'
55
import { requestRunStop } from '@/lib/mothership/async-runs/repository'
66
import { JOB_RUN_TRIGGER } from '@/lib/mothership/jobs/claim'
77
import { dispatchJobOccurrence } from '@/lib/mothership/jobs/dispatch'
@@ -22,7 +22,17 @@ const JOB_RUN_TIME_LIMIT_MS = 6 * 60 * 60_000
2222

2323
const RECONCILE_BATCH_SIZE = 100
2424

25-
function openJobRuns(status: 'queued' | 'running') {
25+
type OpenJobRunStatus = 'queued' | 'running'
26+
27+
/** An occurrence's run record that has not ended; `job_execution_logs_open_idx` covers it. */
28+
export const openJobRun = (statuses: OpenJobRunStatus[]) =>
29+
and(
30+
inArray(jobExecutionLogs.status, statuses),
31+
eq(jobExecutionLogs.trigger, JOB_RUN_TRIGGER),
32+
isNotNull(jobExecutionLogs.occurrenceAt)
33+
)
34+
35+
function openJobRuns(status: OpenJobRunStatus) {
2636
return db
2737
.select({
2838
executionId: jobExecutionLogs.executionId,
@@ -39,13 +49,7 @@ function openJobRuns(status: 'queued' | 'running') {
3949
.from(jobExecutionLogs)
4050
.innerJoin(workflowSchedule, eq(workflowSchedule.id, jobExecutionLogs.scheduleId))
4151
.leftJoin(copilotRuns, eq(copilotRuns.streamId, jobExecutionLogs.executionId))
42-
.where(
43-
and(
44-
eq(jobExecutionLogs.status, status),
45-
eq(jobExecutionLogs.trigger, JOB_RUN_TRIGGER),
46-
isNotNull(jobExecutionLogs.occurrenceAt)
47-
)
48-
)
52+
.where(openJobRun([status]))
4953
.orderBy(asc(jobExecutionLogs.startedAt))
5054
.limit(RECONCILE_BATCH_SIZE)
5155
}

0 commit comments

Comments
 (0)