Repository navigation
Add Kody job update and delete capabilities - #331
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds MCP job-mutation capabilities: ChangesJob Update & Delete Capabilities
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant MCP as MCP Handler (job_update / job_delete)
participant Validator as Zod Validator
participant JobsSvc as Jobs Service
participant DB as Database
participant Logger as Scheduler Logger
Client->>MCP: invoke job_update / job_delete (env, callerContext, args)
MCP->>Validator: validate & normalize input
alt validation fails
Validator-->>MCP: error
MCP-->>Client: reject (schema / mutable-field error)
else
Validator-->>MCP: parsed args
MCP->>JobsSvc: call updateJob() / deleteJob() with mapped payload
alt auth/access denied
JobsSvc-->>MCP: Job not found / access denied
MCP-->>Client: reject "Job <id> was not found."
else
JobsSvc->>DB: mutate / delete job record
DB-->>JobsSvc: result
JobsSvc-->>MCP: updated / deleted result
MCP->>Logger: emit job_updated / job_deleted event
Logger-->>MCP: logged
MCP-->>Client: mapped MCP output (job view / { job_id, deleted: true })
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-331.kentcdodds.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts (1)
184-204: ⚡ Quick winAdd a
job_updatetest case covering aonce-schedule withrun_at→runAttransformation.The existing test exercises a
cronschedule (whereexpressionpasses through unchanged). No test verifies that a caller-facingrun_atinside aonceschedule is correctly transformed torunAtin theupdateJobbody — the same transformation already tested forjob_schedule. If the shared transform helper is ever accidentally gated to the create path only, this path would silently break.🧪 Suggested additional test assertion (inside the existing test or as a new test)
// After the existing cron-schedule test, add a once-schedule variant: +test('job_update transforms run_at to runAt for once schedules', async () => { + resetMocks() + const env = {} as Env + const callerContext = createMcpCallerContext({ + baseUrl: 'https://example.com', + user: { userId: 'user-123', email: 'user@example.com', displayName: 'User Example' }, + }) + mockModule.updateJob.mockResolvedValue({ + id: 'job-123', + name: 'One-off v2', + sourceId: null, + publishedCommit: null, + storageId: 'job:job-123', + schedule: { type: 'once', runAt: '2026-05-01T12:00:00.000Z' }, + scheduleSummary: 'Runs once at 2026-05-01T12:00:00.000Z', + timezone: 'UTC', + enabled: true, + killSwitchEnabled: false, + createdAt: '2026-04-20T10:00:00.000Z', + updatedAt: '2026-04-20T12:00:00.000Z', + nextRunAt: '2026-05-01T12:00:00.000Z', + runCount: 0, + successCount: 0, + errorCount: 0, + runHistory: [], + }) + + await jobUpdateCapability.handler( + { id: 'job-123', schedule: { type: 'once', run_at: '2026-05-01T12:00:00.000Z' } }, + { env, callerContext }, + ) + + expect(mockModule.updateJob).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + schedule: { type: 'once', runAt: '2026-05-01T12:00:00.000Z' }, + }), + }), + ) +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts` around lines 184 - 204, Add a test asserting that jobUpdateCapability.handler transforms a caller-provided once schedule's run_at into runAt in the updateJob payload: call jobUpdateCapability.handler with a schedule.type 'once' and schedule.run_at set (e.g., ISO string), then assert that the mocked updateJob (or the outgoing body) receives schedule.runAt with the same value (and no run_at), ensuring the shared transform used by job_schedule is applied in jobUpdate as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/use/packages.md`:
- Around line 139-143: The docs currently list job_update capabilities but omit
that it can also change a job's name and its kill-switch state; update the docs
text describing job_update to include "name" and "kill-switch" (or equivalent
enabled/disabled/paused flag) alongside the existing mutable fields (schedule,
timezone, enabled state, params, code), matching the implementation in the jobs
shared capability (see job_update handling in the jobs/shared.ts implementation
around the job_update handler). Ensure the wording clarifies that job_update can
rename a job and toggle the kill-switch immediately, and keep the rest of the
listed editable fields as-is.
---
Nitpick comments:
In `@packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts`:
- Around line 184-204: Add a test asserting that jobUpdateCapability.handler
transforms a caller-provided once schedule's run_at into runAt in the updateJob
payload: call jobUpdateCapability.handler with a schedule.type 'once' and
schedule.run_at set (e.g., ISO string), then assert that the mocked updateJob
(or the outgoing body) receives schedule.runAt with the same value (and no
run_at), ensuring the shared transform used by job_schedule is applied in
jobUpdate as well.
🪄 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: d1a20e3a-1d24-46a2-9df3-145620624bf4
📒 Files selected for processing (11)
docs/use/execute.mddocs/use/first-steps.mddocs/use/packages.mdpackages/worker/src/jobs/service.node.test.tspackages/worker/src/mcp/capabilities/jobs/domain.tspackages/worker/src/mcp/capabilities/jobs/job-delete.tspackages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.tspackages/worker/src/mcp/capabilities/jobs/job-update.tspackages/worker/src/mcp/capabilities/jobs/shared.tspackages/worker/src/mcp/jobs-embed.tspackages/worker/src/mcp/server-instructions.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts (1)
721-780:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlso cover
jobScheduleOnceCapabilityin the auth-negative path.This test protects
jobScheduleCapability,jobUpdateCapability,jobDeleteCapability, andjobRunNowCapability, but the one-off scheduling entrypoint is still untested here. A regression in that path would slip through.Suggested addition
await expect( + jobScheduleOnceCapability.handler( + { + code: 'export default async () => ({ ok: true })', + run_at: '2026-04-20T18:30:00Z', + }, + { + env, + callerContext, + }, + ), ).rejects.toThrow('Authenticated MCP user is required for this capability.')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts` around lines 721 - 780, The test is missing the auth-negative check for the one-off scheduler; add a call to jobScheduleOnceCapability.handler with the same unauthenticated callerContext and env (e.g., pass a payload similar to jobScheduleCapability but with schedule: { type: 'once', at: '...' } or appropriate shape), assert it rejects with 'Authenticated MCP user is required for this capability.', and add an expectation that mockModule.createJob (or whichever mock job creation function jobScheduleOnceCapability uses) was not called; place this alongside the existing rejects.toThrow assertions and the final mock non-call assertions (referencing jobScheduleOnceCapability, mockModule.createJob).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts`:
- Around line 721-780: The test is missing the auth-negative check for the
one-off scheduler; add a call to jobScheduleOnceCapability.handler with the same
unauthenticated callerContext and env (e.g., pass a payload similar to
jobScheduleCapability but with schedule: { type: 'once', at: '...' } or
appropriate shape), assert it rejects with 'Authenticated MCP user is required
for this capability.', and add an expectation that mockModule.createJob (or
whichever mock job creation function jobScheduleOnceCapability uses) was not
called; place this alongside the existing rejects.toThrow assertions and the
final mock non-call assertions (referencing jobScheduleOnceCapability,
mockModule.createJob).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 480b19da-2935-49c1-92bc-ab16f31a8437
📒 Files selected for processing (2)
docs/use/packages.mdpackages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/use/packages.md
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
job_updateandjob_deleteto the jobs capability domain and registry metadataBehavior
job_deletehard-deletes a scheduled job by id for the signed-in userjob_updatesupports safe mutable fields only:name,code,params,schedule,timezone,enabled, andkill_switch_enabledjob_get/job_listjob_listandjob_getalready reflect disabled and kill-switched state, so no separate deletion tombstone was introducedIntentionally unsupported
job_updatedoes not expose internal source reassignment or publish metadata fields such assourceId,publishedCommit, orrepoCheckPolicyValidation
npm run test -- packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts packages/worker/src/jobs/service.node.test.tsnpm run test -- packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.tsnpm run test -- packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.tsnpm run typechecknpm run buildnpm run format:check -- docs/use/execute.md docs/use/first-steps.md docs/use/packages.md packages/worker/src/mcp/capabilities/jobs/job-delete.ts packages/worker/src/mcp/capabilities/jobs/job-update.ts packages/worker/src/mcp/capabilities/jobs/shared.ts packages/worker/src/mcp/capabilities/jobs/domain.ts packages/worker/src/mcp/capabilities/jobs/job-schedule.node.test.ts packages/worker/src/jobs/service.node.test.ts packages/worker/src/mcp/jobs-embed.ts packages/worker/src/mcp/server-instructions.tsNotes
job_updaterun_attorunAtjob_schedule_oncealongsidejob_schedule,job_update,job_delete, andjob_run_nownpm run lint -- ...exits successfully in this repo but still reports unrelated pre-existing warnings from untouched files, so I did not treat those as blockers for this jobs-scoped change.Summary by CodeRabbit
New Features
Documentation
Tests