fix: clean up cron job edit payloads (#364)

x找我下，跟你聊聊

packages/client/src/api/hermes/jobs.ts

@@ -1,5 +1,33 @@
 import { request } from '../client'
 
+export interface JobScheduleInterval {
+  kind: 'interval'
+  minutes: number
+  display: string
+}
+
+export interface JobScheduleCron {
+  kind: 'cron'
+  expr: string
+  display: string
+}
+
+export interface JobScheduleOnce {
+  kind: 'once'
+  run_at: string
+  display: string
+}
+
+type UnknownJobSchedule = {
+  kind: string
+  display?: string
+  expr?: string
+  minutes?: number
+  run_at?: string
+}
+
+export type JobSchedule = string | JobScheduleInterval | JobScheduleCron | JobScheduleOnce | UnknownJobSchedule
+
 export interface Job {
   job_id: string
   id: string
@@ -12,7 +40,7 @@ export interface Job {
   provider: string | null
   base_url: string | null
   script: string | null
-  schedule: string | { kind: string; expr: string; display: string }
+  schedule: JobSchedule
   schedule_display: string
   repeat: string | { times: number | null; completed: number }
   enabled: boolean
@@ -45,21 +73,79 @@ export interface CreateJobRequest {
 
 export interface UpdateJobRequest {
   name?: string
-  schedule?: string | { kind: string; expr: string; display: string }
+  schedule?: string
   prompt?: string
   deliver?: string
   skills?: string[]
   skill?: string
-  repeat?: number
+  repeat?: number | null
   enabled?: boolean
   model?: string
   provider?: string
 }
 
+export interface JobFormValues {
+  name: string
+  schedule: string
+  prompt: string
+  deliver: string
+  repeat_times: number | null
+}
+
 function unwrap(res: { job: Job }): Job {
   return res.job
 }
 
+function isScheduleObject(schedule: JobSchedule | null | undefined): schedule is Exclude<JobSchedule, string> {
+  return typeof schedule === 'object' && schedule !== null
+}
+
+export function scheduleToEditableInput(schedule: JobSchedule | null | undefined, fallback = ''): string {
+  if (typeof schedule === 'string') return schedule
+  if (!isScheduleObject(schedule)) return fallback
+
+  if (schedule.kind === 'cron') return schedule.expr || schedule.display || fallback
+  if (schedule.kind === 'once') return schedule.run_at || schedule.display || fallback
+  if (schedule.kind === 'interval') {
+    return schedule.display || (typeof schedule.minutes === 'number' ? `every ${schedule.minutes}m` : fallback)
+  }
+
+  const unknownSchedule = schedule as UnknownJobSchedule
+  return unknownSchedule.expr || unknownSchedule.run_at || unknownSchedule.display || fallback
+}
+
+export function scheduleToDisplayText(schedule: JobSchedule | null | undefined, fallback = '—'): string {
+  if (typeof schedule === 'string') return schedule
+  if (!isScheduleObject(schedule)) return fallback
+
+  if (schedule.kind === 'cron') return schedule.expr || schedule.display || fallback
+  if (schedule.kind === 'interval') return schedule.display || scheduleToEditableInput(schedule, fallback)
+  if (schedule.kind === 'once') return schedule.display || scheduleToEditableInput(schedule, fallback)
+
+  const unknownSchedule = schedule as UnknownJobSchedule
+  return unknownSchedule.display || unknownSchedule.expr || unknownSchedule.run_at || fallback
+}
+
+export function jobRepeatToEditValue(repeat: Job['repeat']): number | null {
+  if (repeat && typeof repeat === 'object') return repeat.times ?? null
+  return null
+}
+
+export function buildJobUpdateRequest(original: Job, form: JobFormValues): UpdateJobRequest {
+  const payload: UpdateJobRequest = {}
+  const originalSchedule = scheduleToEditableInput(original.schedule, original.schedule_display || '')
+  const originalRepeat = jobRepeatToEditValue(original.repeat)
+  const originalDeliver = original.deliver || 'origin'
+
+  if (form.name !== original.name) payload.name = form.name
+  if (form.schedule !== originalSchedule) payload.schedule = form.schedule
+  if (form.prompt !== (original.prompt || '')) payload.prompt = form.prompt
+  if (form.deliver !== originalDeliver) payload.deliver = form.deliver
+  if (form.repeat_times !== originalRepeat) payload.repeat = form.repeat_times
+
+  return payload
+}
+
 export async function listJobs(): Promise<Job[]> {
   const res = await request<{ jobs: Job[] }>('/api/hermes/jobs?include_disabled=true')
   return res.jobs

packages/client/src/components/hermes/jobs/JobCard.vue

@@ -2,6 +2,7 @@
 import { computed } from 'vue'
 import { NButton, NTooltip, useMessage } from 'naive-ui'
 import type { Job } from '@/api/hermes/jobs'
+import { scheduleToDisplayText } from '@/api/hermes/jobs'
 import { useJobsStore } from '@/stores/hermes/jobs'
 import { useI18n } from 'vue-i18n'
 
@@ -35,11 +36,7 @@ const statusType = computed(() => {
   return 'success' as const
 })
 
-const scheduleExpr = computed(() => {
-  const s = props.job.schedule
-  if (typeof s === 'string') return s
-  return s?.expr || props.job.schedule_display || '—'
-})
+const scheduleExpr = computed(() => scheduleToDisplayText(props.job.schedule, props.job.schedule_display || '—'))
 
 const formatTime = (t?: string | null) => {
   if (!t) return '—'

packages/client/src/components/hermes/jobs/JobFormModal.vue

@@ -2,7 +2,13 @@
 import { ref, onMounted, computed } from 'vue'
 import { NModal, NForm, NFormItem, NInput, NButton, NSelect, NInputNumber, useMessage } from 'naive-ui'
 import { useJobsStore } from '@/stores/hermes/jobs'
-import type { CreateJobRequest, UpdateJobRequest } from '@/api/hermes/jobs'
+import {
+  buildJobUpdateRequest,
+  getJob,
+  jobRepeatToEditValue,
+  scheduleToEditableInput,
+} from '@/api/hermes/jobs'
+import type { CreateJobRequest, Job } from '@/api/hermes/jobs'
 import { useI18n } from 'vue-i18n'
 
 const { t } = useI18n()
@@ -49,22 +55,19 @@ const targetOptions = computed(() => [
   { label: t('jobs.local'), value: 'local' },
 ])
 
-const originalSchedule = ref<{ kind: string; expr: string; display: string } | null>(null)
+const originalJob = ref<Job | null>(null)
 
 onMounted(async () => {
   if (props.jobId) {
     try {
-      const { getJob } = await import('@/api/hermes/jobs')
       const job = await getJob(props.jobId)
+      originalJob.value = job
       formData.value = {
         name: job.name,
-        schedule: typeof job.schedule === 'string' ? job.schedule : (job.schedule?.expr || job.schedule_display || ''),
+        schedule: scheduleToEditableInput(job.schedule, job.schedule_display || ''),
         prompt: job.prompt,
         deliver: job.deliver || 'origin',
-        repeat_times: typeof job.repeat === 'number' ? job.repeat : (typeof job.repeat === 'object' ? job.repeat.times : null),
-      }
-      if (typeof job.schedule === 'object' && job.schedule) {
-        originalSchedule.value = job.schedule
+        repeat_times: jobRepeatToEditValue(job.repeat),
       }
     } catch (e: any) {
       message.error(t('jobs.loadFailed') + ': ' + e.message)
@@ -85,20 +88,15 @@ async function handleSave() {
   loading.value = true
   try {
     if (isEdit.value) {
-      const payload: UpdateJobRequest = {
-        name: formData.value.name,
-        prompt: formData.value.prompt,
-        deliver: formData.value.deliver,
-        repeat: formData.value.repeat_times ?? undefined,
+      if (!originalJob.value) {
+        message.error(t('jobs.loadFailed'))
+        return
       }
-      if (originalSchedule.value) {
-        payload.schedule = {
-          kind: originalSchedule.value.kind,
-          expr: formData.value.schedule,
-          display: formData.value.schedule,
-        }
-      } else {
-        payload.schedule = formData.value.schedule
+      const payload = buildJobUpdateRequest(originalJob.value, formData.value)
+      if (Object.keys(payload).length === 0) {
+        message.success(t('jobs.jobUpdated'))
+        emit('saved')
+        return
       }
       await jobsStore.updateJob(props.jobId!, payload)
       message.success(t('jobs.jobUpdated'))

packages/server/src/controllers/hermes/jobs.ts

@@ -25,6 +25,20 @@ function buildHeaders(profile: string): Record<string, string> {
 
 const TIMEOUT_MS = 30_000
 
+async function readUpstreamError(res: Response): Promise<unknown> {
+  const contentType = res.headers.get('content-type') || ''
+  if (contentType.includes('application/json')) {
+    try {
+      return await res.json()
+    } catch {
+      // Fall through to a stable error shape below.
+    }
+  }
+
+  const text = await res.text().catch(() => '')
+  return { error: { message: text || `Upstream error: ${res.status} ${res.statusText}` } }
+}
+
 async function proxyRequest(ctx: Context, upstreamPath: string, method?: string): Promise<void> {
   const profile = resolveProfile(ctx)
   const upstream = getUpstream(profile)
@@ -38,17 +52,25 @@ async function proxyRequest(ctx: Context, upstreamPath: string, method?: string)
     ? JSON.stringify(ctx.request.body || {})
     : undefined
 
-  const res = await fetch(url, {
-    method: method || ctx.req.method,
-    headers,
-    body,
-    signal: AbortSignal.timeout(TIMEOUT_MS),
-  })
-
-  if (!res.ok) {
+  let res: Response
+  try {
+    res = await fetch(url, {
+      method: method || ctx.req.method,
+      headers,
+      body,
+      signal: AbortSignal.timeout(TIMEOUT_MS),
+    })
+  } catch (e: any) {
     ctx.status = 502
     ctx.set('Content-Type', 'application/json')
-    ctx.body = { error: { message: `Upstream error: ${res.status} ${res.statusText}` } }
+    ctx.body = { error: { message: `Proxy error: ${e.message}` } }
+    return
+  }
+
+  if (!res.ok) {
+    ctx.status = res.status
+    ctx.set('Content-Type', 'application/json')
+    ctx.body = await readUpstreamError(res)
     return
   }
 

tests/client/jobs.test.ts

@@ -0,0 +1,121 @@
+// @vitest-environment jsdom
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mockFetch = vi.fn()
+vi.stubGlobal('fetch', mockFetch)
+
+vi.mock('@/router', () => ({
+  default: {
+    currentRoute: { value: { name: 'hermes.jobs' } },
+    replace: vi.fn(),
+  },
+}))
+
+import {
+  buildJobUpdateRequest,
+  scheduleToDisplayText,
+  scheduleToEditableInput,
+  updateJob,
+} from '../../packages/client/src/api/hermes/jobs'
+import type { Job } from '../../packages/client/src/api/hermes/jobs'
+
+function makeJob(overrides: Partial<Job> = {}): Job {
+  return {
+    job_id: 'job-1',
+    id: 'job-1',
+    name: 'artifact cleanup',
+    prompt: 'short prompt',
+    skills: [],
+    skill: null,
+    model: null,
+    provider: null,
+    base_url: null,
+    script: null,
+    schedule: { kind: 'interval', minutes: 7200, display: 'every 7200m' },
+    schedule_display: 'every 7200m',
+    repeat: { times: null, completed: 0 },
+    enabled: true,
+    state: 'scheduled',
+    paused_at: null,
+    paused_reason: null,
+    created_at: '2026-04-30T00:00:00Z',
+    next_run_at: null,
+    last_run_at: null,
+    last_status: null,
+    last_error: null,
+    deliver: 'origin',
+    origin: null,
+    last_delivery_error: null,
+    ...overrides,
+  }
+}
+
+describe('Hermes jobs edit payloads', () => {
+  beforeEach(() => {
+    localStorage.clear()
+    vi.clearAllMocks()
+  })
+
+  it('uses display text for interval schedules without manufacturing expr', () => {
+    const schedule = { kind: 'interval' as const, minutes: 7200, display: 'every 7200m' }
+
+    expect(scheduleToEditableInput(schedule, '')).toBe('every 7200m')
+    expect(scheduleToDisplayText(schedule, '—')).toBe('every 7200m')
+  })
+
+  it('keeps cron expr as the editable schedule string', () => {
+    const schedule = { kind: 'cron' as const, expr: '0 9 * * 1', display: '0 9 * * 1' }
+
+    expect(scheduleToEditableInput(schedule, '')).toBe('0 9 * * 1')
+    expect(scheduleToDisplayText(schedule, '—')).toBe('0 9 * * 1')
+  })
+
+  it('omits unchanged long prompts from name-only updates', () => {
+    const prompt = 'x'.repeat(9484)
+    const original = makeJob({ prompt })
+
+    const payload = buildJobUpdateRequest(original, {
+      name: 'artifact cleanup renamed',
+      schedule: 'every 7200m',
+      prompt,
+      deliver: 'origin',
+      repeat_times: null,
+    })
+
+    expect(payload).toEqual({ name: 'artifact cleanup renamed' })
+    expect(payload).not.toHaveProperty('prompt')
+    expect(payload).not.toHaveProperty('schedule')
+  })
+
+  it('sends changed interval schedules as raw strings', () => {
+    const original = makeJob()
+
+    const payload = buildJobUpdateRequest(original, {
+      name: original.name,
+      schedule: 'every 14400m',
+      prompt: original.prompt,
+      deliver: 'origin',
+      repeat_times: null,
+    })
+
+    expect(payload).toEqual({ schedule: 'every 14400m' })
+  })
+
+  it('does not send a PATCH body with structured schedule objects', async () => {
+    const returnedJob = makeJob({ name: 'artifact cleanup renamed' })
+    mockFetch.mockResolvedValue({
+      ok: true,
+      status: 200,
+      json: () => Promise.resolve({ job: returnedJob }),
+    })
+
+    await updateJob('job-1', { name: 'artifact cleanup renamed', schedule: 'every 14400m' })
+
+    expect(mockFetch).toHaveBeenCalledOnce()
+    const [, options] = mockFetch.mock.calls[0]
+    expect(JSON.parse(options.body as string)).toEqual({
+      name: 'artifact cleanup renamed',
+      schedule: 'every 14400m',
+    })
+  })
+})

tests/server/jobs-controller.test.ts

@@ -0,0 +1,68 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('../../packages/server/src/config', () => ({
+  config: { upstream: 'http://127.0.0.1:8642' },
+}))
+
+vi.mock('../../packages/server/src/services/gateway-bootstrap', () => ({
+  getGatewayManagerInstance: () => null,
+}))
+
+const mockFetch = vi.fn()
+vi.stubGlobal('fetch', mockFetch)
+
+import { update } from '../../packages/server/src/controllers/hermes/jobs'
+
+function createMockCtx(overrides: Record<string, any> = {}) {
+  const ctx: any = {
+    req: { method: 'PATCH' },
+    request: { body: { name: 'renamed' } },
+    params: { id: 'abc123abc123' },
+    query: {},
+    search: '',
+    headers: {},
+    status: 200,
+    set: vi.fn(),
+    body: null,
+    ...overrides,
+  }
+  ctx.get = (name: string) => {
+    const match = Object.entries(ctx.headers).find(([key]) => key.toLowerCase() === name.toLowerCase())
+    const value = match?.[1]
+    return Array.isArray(value) ? value[0] : value || ''
+  }
+  return ctx
+}
+
+describe('Hermes jobs controller proxy', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('passes through upstream validation status and body instead of masking it as 502', async () => {
+    mockFetch.mockResolvedValue({
+      ok: false,
+      status: 400,
+      statusText: 'Bad Request',
+      headers: new Headers({ 'content-type': 'application/json' }),
+      json: () => Promise.resolve({ error: 'Prompt must be ≤ 5000 characters' }),
+    })
+
+    const ctx = createMockCtx()
+    await update(ctx)
+
+    expect(ctx.status).toBe(400)
+    expect(ctx.body).toEqual({ error: 'Prompt must be ≤ 5000 characters' })
+    expect(ctx.set).toHaveBeenCalledWith('Content-Type', 'application/json')
+  })
+
+  it('keeps real proxy connection failures as 502', async () => {
+    mockFetch.mockRejectedValue(new Error('ECONNREFUSED'))
+
+    const ctx = createMockCtx()
+    await update(ctx)
+
+    expect(ctx.status).toBe(502)
+    expect(ctx.body).toEqual({ error: { message: 'Proxy error: ECONNREFUSED' } })
+  })
+})