diff --git a/packages/client/src/api/hermes/jobs.ts b/packages/client/src/api/hermes/jobs.ts index 161e4f6..a068d36 100644 --- a/packages/client/src/api/hermes/jobs.ts +++ b/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 { + 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 { const res = await request<{ jobs: Job[] }>('/api/hermes/jobs?include_disabled=true') return res.jobs diff --git a/packages/client/src/components/hermes/jobs/JobCard.vue b/packages/client/src/components/hermes/jobs/JobCard.vue index 1d91a08..adefc10 100644 --- a/packages/client/src/components/hermes/jobs/JobCard.vue +++ b/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 '—' diff --git a/packages/client/src/components/hermes/jobs/JobFormModal.vue b/packages/client/src/components/hermes/jobs/JobFormModal.vue index f52e146..2351253 100644 --- a/packages/client/src/components/hermes/jobs/JobFormModal.vue +++ b/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(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')) diff --git a/packages/server/src/controllers/hermes/jobs.ts b/packages/server/src/controllers/hermes/jobs.ts index ec12c8d..0e0249b 100644 --- a/packages/server/src/controllers/hermes/jobs.ts +++ b/packages/server/src/controllers/hermes/jobs.ts @@ -25,6 +25,20 @@ function buildHeaders(profile: string): Record { const TIMEOUT_MS = 30_000 +async function readUpstreamError(res: Response): Promise { + 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 { 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 } diff --git a/tests/client/jobs.test.ts b/tests/client/jobs.test.ts new file mode 100644 index 0000000..5fcfe15 --- /dev/null +++ b/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 { + 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', + }) + }) +}) diff --git a/tests/server/jobs-controller.test.ts b/tests/server/jobs-controller.test.ts new file mode 100644 index 0000000..7f8a3be --- /dev/null +++ b/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 = {}) { + 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' } }) + }) +})