diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index e0a7216abc..12160a7823 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -34,7 +34,7 @@ import { playWakeSound } from '@/lib/wake-sound' import { $billingSettingsRequest } from '@/store/billing-block' import { $desktopBoot } from '@/store/boot' import { requestVoiceConversationStart } from '@/store/composer' -import { setCronFocusJobId } from '@/store/cron' +import { $cronReviewRequest, setCronFocusJobId } from '@/store/cron' import { $pinnedSessionIds, pinSession, restoreWorktree, unpinSession } from '@/store/layout' import { $previewTarget } from '@/store/preview' import { @@ -158,6 +158,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { // context (the sticky toast). The shell owns `navigate`, so it consumes the // intent counter here; the ref skips the initial mount value. const billingSettingsSeenRef = useRef(0) + const cronReviewSeenRef = useRef(0) const messagingTranscriptSignatureRef = useRef(new Map()) // Stable identity for the whole callback surface (see WiringActions). Mutated // in place each render so memoized surfaces never re-render on churn. @@ -166,6 +167,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { const gatewayState = useStore($gatewayState) const activeSessionId = useStore($activeSessionId) const billingSettingsRequest = useStore($billingSettingsRequest) + const cronReviewRequest = useStore($cronReviewRequest) const currentCwd = useStore($currentCwd) // eslint-disable-next-line no-restricted-syntax -- one-shot request-seen sentinel, not an atom mirror @@ -180,6 +182,19 @@ export function ContribWiring({ children }: { children: ReactNode }) { navigate(`${SETTINGS_ROUTE}?tab=billing`) } }, [billingSettingsRequest, navigate]) + + // eslint-disable-next-line no-restricted-syntax -- one-shot request-seen sentinel, not an atom mirror + useEffect(() => { + if (cronReviewRequest === cronReviewSeenRef.current) { + return + } + + cronReviewSeenRef.current = cronReviewRequest + + if (cronReviewRequest > 0) { + navigate(CRON_ROUTE) + } + }, [cronReviewRequest, navigate]) const freshDraftReady = useStore($freshDraftReady) const resumeFailedSessionId = useStore($resumeFailedSessionId) const resumeExhaustedSessionId = useStore($resumeExhaustedSessionId) diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index b5c09587a5..5343e2ef00 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -31,6 +31,7 @@ vi.mock('@/hermes', () => ({ getGlobalModelInfo: () => getGlobalModelInfo(), getGlobalModelOptions: () => getGlobalModelOptions(), getAuxiliaryModels: () => getAuxiliaryModels(), + getApiRequestProfile: () => 'default', getMoaModels: () => getMoaModels(), setModelAssignment: (body: unknown) => setModelAssignment(body), getRecommendedDefaultModel: (slug: string) => getRecommendedDefaultModel(slug), @@ -71,7 +72,7 @@ beforeEach(() => { tasks: [{ task: 'vision', provider: 'auto', model: '', base_url: '' }] }) getMoaModels.mockResolvedValue(null) - setModelAssignment.mockResolvedValue({ provider: 'nous', model: 'hermes-4', gateway_tools: [] }) + setModelAssignment.mockResolvedValue({ ok: true, provider: 'nous', model: 'hermes-4', gateway_tools: [] }) getRecommendedDefaultModel.mockResolvedValue({ provider: 'nous', model: 'hermes-4', free_tier: null }) setEnvVar.mockResolvedValue({ ok: true }) getHermesConfigRecord.mockResolvedValue({ agent: { reasoning_effort: 'medium', service_tier: 'normal' } }) @@ -232,6 +233,7 @@ describe('ModelSettings', () => { ] }) setModelAssignment.mockResolvedValueOnce({ + ok: true, provider: 'local-ollama', model: 'qwen3:latest', gateway_tools: [] @@ -353,6 +355,7 @@ describe('ModelSettings', () => { it('warns when a main switch leaves auxiliary tasks pinned to another provider', async () => { setModelAssignment.mockResolvedValueOnce({ + ok: true, provider: 'openrouter', model: 'anthropic/claude-opus-4.7', gateway_tools: [], diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index fd19d165c4..b91a04f103 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -27,6 +27,7 @@ import { useI18n } from '@/i18n' import { AlertTriangle, Cpu, Loader2 } from '@/lib/icons' import { DEFAULT_REASONING_EFFORT, REASONING_EFFORT_VALUES } from '@/lib/reasoning-effort' import { cn } from '@/lib/utils' +import { setMainModelAssignment } from '@/store/cron-model-impact' import { notifyError } from '@/store/notifications' import { startManualLocalEndpoint, startManualOnboarding, startManualProviderOAuth } from '@/store/onboarding' @@ -619,10 +620,9 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { setError('') try { - const result = await setModelAssignment({ + const result = await setMainModelAssignment({ model: selectedModel, provider: selectedProvider, - scope: 'main', ...(selectedProviderRow?.api_url ? { base_url: selectedProviderRow.api_url } : {}) }) diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index c1d1aa6b88..720904b063 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1370,6 +1370,13 @@ export const ar = defineLocale({ }, cron: { close: 'إغلاق', + modelImpact: { + title: 'تحتاج المهام المجدولة إلى المراجعة', + message: count => `سيتم تخطي ${count} من المهام المجدولة حتى تراجع إعدادات النموذج الخاصة بها.`, + detailMore: (names, remaining) => `${names} و${remaining} أخرى`, + review: 'مراجعة المهام المجدولة', + saveFailed: 'لم يحفظ Hermes تغيير النموذج هذا.' + }, search: 'بحث', loading: 'جار التحميل...', states: { diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index ad74192899..965200df60 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1666,6 +1666,14 @@ export const en: Translations = { close: 'Close cron', title: 'Scheduled jobs', count: count => `${count} ${count === 1 ? 'job' : 'jobs'}`, + modelImpact: { + title: 'Scheduled jobs need review', + message: count => + `${count} scheduled ${count === 1 ? 'job' : 'jobs'} will be skipped until you review their model settings.`, + detailMore: (names, remaining) => `${names} and ${remaining} more`, + review: 'Review scheduled jobs', + saveFailed: 'Hermes did not save that model change.' + }, search: 'Search cron jobs...', loading: 'Loading cron jobs...', states: { diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 2cb2ccb820..6ff8e056ac 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1489,6 +1489,13 @@ export const ja = defineLocale({ close: 'Cron を閉じる', title: 'スケジュール済みジョブ', count: count => `${count} 件のジョブ`, + modelImpact: { + title: 'スケジュール済みジョブの確認が必要です', + message: count => `モデル設定を確認するまで、${count} 件のスケジュール済みジョブがスキップされます。`, + detailMore: (names, remaining) => `${names}、ほか ${remaining} 件`, + review: 'スケジュール済みジョブを確認', + saveFailed: 'Hermes はモデルの変更を保存しませんでした。' + }, search: 'Cron ジョブを検索...', loading: 'Cron ジョブを読み込み中...', states: { diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index cacae65499..8752503ab9 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1403,6 +1403,13 @@ export interface Translations { close: string title: string count: (count: number) => string + modelImpact: { + title: string + message: (count: number) => string + detailMore: (names: string, remaining: number) => string + review: string + saveFailed: string + } search: string loading: string states: Record diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 97d85149eb..9e1bfad1cc 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1436,6 +1436,13 @@ export const zhHant = defineLocale({ close: '關閉排程', title: '排程工作', count: count => `${count} 個工作`, + modelImpact: { + title: '排程工作需要檢查', + message: count => `在您檢查模型設定之前，${count} 個排程工作將被略過。`, + detailMore: (names, remaining) => `${names}，以及另外 ${remaining} 個`, + review: '檢查排程工作', + saveFailed: 'Hermes 未儲存該模型變更。' + }, search: '搜尋排程工作…', loading: '正在載入排程工作…', states: { diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 5bd1a134e8..347132690b 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -1858,6 +1858,13 @@ export const zh: Translations = { close: '关闭定时任务', title: '定时任务', count: count => `${count} 个任务`, + modelImpact: { + title: '定时任务需要检查', + message: count => `在您检查模型设置之前，${count} 个定时任务将被跳过。`, + detailMore: (names, remaining) => `${names}，以及另外 ${remaining} 个`, + review: '检查定时任务', + saveFailed: 'Hermes 未保存该模型更改。' + }, search: '搜索定时任务…', loading: '正在加载定时任务…', states: { diff --git a/apps/desktop/src/store/cron-model-impact-scope.test.ts b/apps/desktop/src/store/cron-model-impact-scope.test.ts new file mode 100644 index 0000000000..1bc9af64ad --- /dev/null +++ b/apps/desktop/src/store/cron-model-impact-scope.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' + +import type { HermesConnection } from '@/global' + +import { getCronModelImpactScope, syncCronModelImpactConnection } from './cron-model-impact-scope' + +function connection( + baseUrl: string, + wsUrl: string, + overrides: Partial = {} +): HermesConnection { + return { + baseUrl, + isFullscreen: false, + mode: 'remote', + nativeOverlayWidth: 0, + token: 'secret-not-part-of-identity', + wsUrl, + logs: [], + windowButtonPosition: null, + ...overrides + } +} + +describe('cron model impact backend identity', () => { + it('survives disconnects and reminted websocket tickets but invalidates for another backend', () => { + syncCronModelImpactConnection(connection('https://one.example', 'wss://one.example?ticket=first')) + const first = getCronModelImpactScope() + + syncCronModelImpactConnection(null) + expect(getCronModelImpactScope()).toEqual(first) + + syncCronModelImpactConnection(connection('https://one.example', 'wss://one.example?ticket=second')) + expect(getCronModelImpactScope()).toEqual(first) + + syncCronModelImpactConnection(connection('https://two.example', 'wss://two.example?ticket=third')) + expect(getCronModelImpactScope().generation).toBe(first.generation + 1) + }) + + it('treats reminted SSH tunnel ports as the same remote backend', () => { + const ssh = { + remoteKind: 'ssh' as const, + remoteIdentity: 'operator@remote-box', + remoteHost: 'remote-box' + } + syncCronModelImpactConnection(connection('http://127.0.0.1:41001', 'ws://127.0.0.1:41001', ssh)) + const first = getCronModelImpactScope() + + syncCronModelImpactConnection(connection('http://127.0.0.1:52002', 'ws://127.0.0.1:52002', ssh)) + + expect(getCronModelImpactScope()).toEqual(first) + }) +}) diff --git a/apps/desktop/src/store/cron-model-impact-scope.ts b/apps/desktop/src/store/cron-model-impact-scope.ts new file mode 100644 index 0000000000..25f43e7b88 --- /dev/null +++ b/apps/desktop/src/store/cron-model-impact-scope.ts @@ -0,0 +1,65 @@ +import type { HermesConnection } from '@/global' + +let generation = 0 +let connectionIdentity = '' +const invalidationListeners = new Set<() => void>() + +export interface CronModelImpactScopeSnapshot { + connection: string + generation: number +} + +export function getCronModelImpactScope(): CronModelImpactScopeSnapshot { + return { connection: connectionIdentity, generation } +} + +export function beginCronModelImpactAssignment(): CronModelImpactScopeSnapshot { + generation += 1 + + return getCronModelImpactScope() +} + +export function invalidateCronModelImpactScopeState(): void { + generation += 1 + invalidationListeners.forEach(listener => listener()) +} + +export function onCronModelImpactScopeInvalidated(listener: () => void): () => void { + invalidationListeners.add(listener) + + return () => invalidationListeners.delete(listener) +} + +function identityForConnection(connection: HermesConnection | null): string { + if (!connection) { + return '' + } + + const backendIdentity = + connection.remoteKind === 'ssh' + ? connection.remoteIdentity || connection.remoteHost || '' + : connection.remoteIdentity || connection.baseUrl + + return [connection.mode ?? '', connection.remoteKind ?? '', backendIdentity, connection.profile ?? ''].join( + '\u0000' + ) +} + +/** Keep pending responses and action closures bound to the backend that issued + * them without storing or comparing connection secrets. */ +export function syncCronModelImpactConnection(connection: HermesConnection | null): void { + // A null descriptor is an ordinary reconnect state, not evidence that the + // user selected another backend. Retain the last durable identity so the + // reconnect can prove whether the backend actually changed. + if (!connection) { + return + } + + const next = identityForConnection(connection) + + if (connectionIdentity && connectionIdentity !== next) { + invalidateCronModelImpactScopeState() + } + + connectionIdentity = next +} diff --git a/apps/desktop/src/store/cron-model-impact.test.ts b/apps/desktop/src/store/cron-model-impact.test.ts new file mode 100644 index 0000000000..aaaac73b33 --- /dev/null +++ b/apps/desktop/src/store/cron-model-impact.test.ts @@ -0,0 +1,207 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { $cronReviewRequest } from '@/store/cron' +import { $notifications, clearNotifications, dismissNotification } from '@/store/notifications' +import type { ModelAssignmentResponse } from '@/types/hermes' + +const setModelAssignment = vi.fn() +const getApiRequestProfile = vi.fn<() => string | null>(() => 'default') + +vi.mock('@/hermes', () => ({ + setModelAssignment: (...args: unknown[]) => setModelAssignment(...args), + getApiRequestProfile: () => getApiRequestProfile() +})) + +import { + CRON_MODEL_IMPACT_NOTIFICATION_ID, + invalidateCronModelImpactScope, + setMainModelAssignment +} from '@/store/cron-model-impact' + +function response( + impact: ModelAssignmentResponse['cron_model_impact'] +): ModelAssignmentResponse { + return { + ok: true, + scope: 'main', + provider: 'nous', + model: 'new/model', + cron_model_impact: impact + } +} + +function positive(name = 'Morning summary'): ModelAssignmentResponse['cron_model_impact'] { + return { + available: true, + guard_enabled: true, + affected_count: 1, + truncated: false, + jobs: [{ id: 'job-1', name, drifted_axes: ['provider', 'model'] }] + } +} + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(res => { + resolve = res + }) + return { promise, resolve } +} + +beforeEach(() => { + setModelAssignment.mockReset() + getApiRequestProfile.mockReset() + getApiRequestProfile.mockReturnValue('default') + clearNotifications() + invalidateCronModelImpactScope({ clearNotification: false }) +}) + +describe('setMainModelAssignment', () => { + it('shows one consumer warning and routes via a read-only review action', async () => { + setModelAssignment.mockResolvedValue(response(positive())) + const requestCount = $cronReviewRequest.get() + + await setMainModelAssignment({ provider: 'nous', model: 'new/model' }) + + expect(setModelAssignment).toHaveBeenCalledWith({ + scope: 'main', + provider: 'nous', + model: 'new/model' + }) + const notification = $notifications.get().find(item => item.id === CRON_MODEL_IMPACT_NOTIFICATION_ID) + expect(notification?.kind).toBe('warning') + expect(notification?.title).toBe('Scheduled jobs need review') + expect(notification?.message).toContain('1 scheduled job will be skipped') + expect(notification?.detail).toContain('Morning summary') + expect(notification?.action?.label).toBe('Review scheduled jobs') + + notification?.action?.onClick() + expect($cronReviewRequest.get()).toBe(requestCount + 1) + expect(setModelAssignment).toHaveBeenCalledTimes(1) + }) + + it('ignores malformed untrusted impact data', async () => { + setModelAssignment.mockResolvedValue( + response({ + available: true, + guard_enabled: true, + affected_count: 2, + truncated: false, + jobs: [{ id: 'job-1', name: 'One', drifted_axes: ['provider'] }] + }) + ) + + await setMainModelAssignment({ provider: 'nous', model: 'new/model' }) + + expect($notifications.get()).toEqual([]) + }) + + it('keeps an existing warning for an older backend but clears it on explicit zero impact', async () => { + setModelAssignment.mockResolvedValueOnce(response(positive())) + await setMainModelAssignment({ provider: 'nous', model: 'one' }) + expect($notifications.get()).toHaveLength(1) + + setModelAssignment.mockResolvedValueOnce(response(undefined)) + await setMainModelAssignment({ provider: 'nous', model: 'two' }) + expect($notifications.get()).toHaveLength(1) + const retainedAction = $notifications.get()[0].action + const reviewCount = $cronReviewRequest.get() + retainedAction?.onClick() + expect($cronReviewRequest.get()).toBe(reviewCount + 1) + + setModelAssignment.mockResolvedValueOnce( + response({ + available: true, + guard_enabled: true, + affected_count: 0, + truncated: false, + jobs: [] + }) + ) + await setMainModelAssignment({ provider: 'nous', model: 'three' }) + expect($notifications.get()).toEqual([]) + }) + + it('rejects non-persisted confirmation outcomes without changing impact state', async () => { + setModelAssignment.mockResolvedValueOnce(response(positive())) + await setMainModelAssignment({ provider: 'nous', model: 'one' }) + + setModelAssignment.mockResolvedValueOnce({ + ok: false, + scope: 'main', + provider: 'openrouter', + model: 'openai/gpt-5.5-pro', + confirm_required: true, + confirm_message: 'Confirm this expensive model.' + } satisfies ModelAssignmentResponse) + + await expect( + setMainModelAssignment({ provider: 'openrouter', model: 'openai/gpt-5.5-pro' }) + ).rejects.toThrow('Confirm this expensive model.') + expect($notifications.get()).toHaveLength(1) + const action = $notifications.get()[0].action + const reviewCount = $cronReviewRequest.get() + action?.onClick() + expect($cronReviewRequest.get()).toBe(reviewCount + 1) + }) + + it('publishes only the latest same-profile assignment when responses reverse', async () => { + const first = deferred() + const second = deferred() + setModelAssignment.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise) + + const firstCall = setMainModelAssignment({ provider: 'nous', model: 'first' }) + const secondCall = setMainModelAssignment({ provider: 'nous', model: 'second' }) + second.resolve(response(positive('Second job'))) + await secondCall + first.resolve(response(positive('Stale first job'))) + await firstCall + + const notification = $notifications.get()[0] + expect(notification.detail).toContain('Second job') + expect(notification.detail).not.toContain('Stale first job') + }) + + it('invalidates pending responses and action closures on profile or connection changes', async () => { + const pending = deferred() + setModelAssignment.mockReturnValueOnce(pending.promise) + const call = setMainModelAssignment({ provider: 'nous', model: 'pending' }) + + invalidateCronModelImpactScope() + pending.resolve(response(positive('Stale job'))) + await call + expect($notifications.get()).toEqual([]) + + setModelAssignment.mockResolvedValueOnce(response(positive('Current job'))) + await setMainModelAssignment({ provider: 'nous', model: 'current' }) + const action = $notifications.get()[0].action + const requestCount = $cronReviewRequest.get() + getApiRequestProfile.mockReturnValue('other') + invalidateCronModelImpactScope({ clearNotification: false }) + action?.onClick() + expect($cronReviewRequest.get()).toBe(requestCount) + }) + + it('clears an obsolete warning when the drift guard is disabled', async () => { + setModelAssignment.mockResolvedValueOnce(response(positive())) + await setMainModelAssignment({ provider: 'nous', model: 'one' }) + expect($notifications.get()).toHaveLength(1) + + setModelAssignment.mockResolvedValueOnce( + response({ available: true, guard_enabled: false, affected_count: 0, truncated: false, jobs: [] }) + ) + await setMainModelAssignment({ provider: 'nous', model: 'two' }) + + expect($notifications.get()).toEqual([]) + }) + + it('does not let a dismissed notification mutate cron configuration', async () => { + setModelAssignment.mockResolvedValue(response(positive())) + await setMainModelAssignment({ provider: 'nous', model: 'new/model' }) + + dismissNotification(CRON_MODEL_IMPACT_NOTIFICATION_ID) + + expect($notifications.get()).toEqual([]) + expect(setModelAssignment).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/desktop/src/store/cron-model-impact.ts b/apps/desktop/src/store/cron-model-impact.ts new file mode 100644 index 0000000000..27fe59e219 --- /dev/null +++ b/apps/desktop/src/store/cron-model-impact.ts @@ -0,0 +1,195 @@ +import { getApiRequestProfile, setModelAssignment } from '@/hermes' +import { translateNow } from '@/i18n' +import { requestCronReview } from '@/store/cron' +import { + beginCronModelImpactAssignment, + getCronModelImpactScope, + invalidateCronModelImpactScopeState, + onCronModelImpactScopeInvalidated +} from '@/store/cron-model-impact-scope' +import { dismissNotification, notify } from '@/store/notifications' +import type { + CronModelDriftAxis, + CronModelImpact, + CronModelImpactJob, + ModelAssignmentRequest, + ModelAssignmentResponse +} from '@/types/hermes' + +export const CRON_MODEL_IMPACT_NOTIFICATION_ID = 'cron-model-impact' + +const MAX_JOBS = 50 +const MAX_ID_CODE_POINTS = 256 +const MAX_NAME_CODE_POINTS = 120 +const ALLOWED_AXES = new Set(['provider', 'model']) + + +function profileIdentity(): string { + return getApiRequestProfile()?.trim() || 'default' +} + +function codePointLength(value: string): number { + return [...value].length +} + +function hasControlCharacters(value: string): boolean { + return /\p{C}/u.test(value) +} + +function validJob(value: unknown): value is CronModelImpactJob { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false + } + + const job = value as Partial + + if ( + typeof job.id !== 'string' || + job.id.trim() !== job.id || + !job.id || + codePointLength(job.id) > MAX_ID_CODE_POINTS || + hasControlCharacters(job.id) || + typeof job.name !== 'string' || + job.name.trim() !== job.name || + !job.name || + codePointLength(job.name) > MAX_NAME_CODE_POINTS || + hasControlCharacters(job.name) || + !Array.isArray(job.drifted_axes) || + job.drifted_axes.length < 1 || + job.drifted_axes.length > 2 || + new Set(job.drifted_axes).size !== job.drifted_axes.length || + !job.drifted_axes.every(axis => ALLOWED_AXES.has(axis)) + ) { + return false + } + + return true +} + +export function parseCronModelImpact(value: unknown): CronModelImpact | null { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return null + } + + const impact = value as Partial + + if ( + typeof impact.available !== 'boolean' || + typeof impact.guard_enabled !== 'boolean' || + !Number.isSafeInteger(impact.affected_count) || + (impact.affected_count ?? -1) < 0 || + typeof impact.truncated !== 'boolean' || + !Array.isArray(impact.jobs) || + impact.jobs.length > MAX_JOBS || + !impact.jobs.every(validJob) + ) { + return null + } + + const count = impact.affected_count as number + const ids = impact.jobs.map(job => job.id) + + if ( + new Set(ids).size !== ids.length || + (!impact.truncated && count !== impact.jobs.length) || + (impact.truncated && (impact.jobs.length !== MAX_JOBS || count <= impact.jobs.length)) + ) { + return null + } + + return impact as CronModelImpact +} + +function currentResponseScope(profile: string, connection: string, generation: number): boolean { + const scope = getCronModelImpactScope() + + return profileIdentity() === profile && scope.connection === connection && scope.generation === generation +} + +function currentActionScope(profile: string, connection: string): boolean { + return profileIdentity() === profile && getCronModelImpactScope().connection === connection +} + +function detailFor(impact: CronModelImpact): string { + const visible = impact.jobs.slice(0, 3).map(job => job.name) + const remaining = impact.affected_count - visible.length + + return remaining > 0 + ? translateNow('cron.modelImpact.detailMore', visible.join(', '), remaining) + : visible.join(', ') +} + +function publishImpact( + impact: CronModelImpact, + profile: string, + connection: string, + generation: number +): void { + if (!impact.available) { + return + } + + if (!impact.guard_enabled || impact.affected_count === 0) { + dismissNotification(CRON_MODEL_IMPACT_NOTIFICATION_ID) + + return + } + + notify({ + id: CRON_MODEL_IMPACT_NOTIFICATION_ID, + kind: 'warning', + title: translateNow('cron.modelImpact.title'), + message: translateNow('cron.modelImpact.message', impact.affected_count), + detail: detailFor(impact), + action: { + label: translateNow('cron.modelImpact.review'), + onClick: () => { + if (currentActionScope(profile, connection)) { + requestCronReview() + } + } + } + }) +} + +export async function setMainModelAssignment( + request: Omit +): Promise { + const { connection, generation } = beginCronModelImpactAssignment() + const profile = profileIdentity() + const result = await setModelAssignment({ ...request, scope: 'main' }) + + if (result.ok !== true) { + throw new Error(result.confirm_message?.trim() || translateNow('cron.modelImpact.saveFailed')) + } + + if (!currentResponseScope(profile, connection, generation)) { + return result + } + + // Missing means an older backend. It is not evidence that an existing impact + // has gone away, so leave the current warning untouched. + if (result.cron_model_impact !== undefined) { + const impact = parseCronModelImpact(result.cron_model_impact) + + if (impact) { + publishImpact(impact, profile, connection, generation) + } + } + + return result +} + +export function invalidateCronModelImpactScope(options: { clearNotification?: boolean } = {}): void { + if (options.clearNotification === false) { + beginCronModelImpactAssignment() + + return + } + + invalidateCronModelImpactScopeState() +} + +// Scope changes originating outside this module (profile/backend switches) +// clear any warning that belongs to the old runtime. +onCronModelImpactScopeInvalidated(() => dismissNotification(CRON_MODEL_IMPACT_NOTIFICATION_ID)) diff --git a/apps/desktop/src/store/cron.ts b/apps/desktop/src/store/cron.ts index 2c492b3490..f017f60dc1 100644 --- a/apps/desktop/src/store/cron.ts +++ b/apps/desktop/src/store/cron.ts @@ -17,3 +17,8 @@ export const updateCronJobs = (fn: (jobs: CronJob[]) => CronJob[]) => $cronJobs. // after consumption so re-opening cron normally doesn't re-focus a stale job. export const $cronFocusJobId = atom(null) export const setCronFocusJobId = (id: null | string) => $cronFocusJobId.set(id) + +// Shell-owned one-shot intent for stores without router context. Do not set a +// focus id here: the cron overlay's first fetch may not have loaded that row. +export const $cronReviewRequest = atom(0) +export const requestCronReview = () => $cronReviewRequest.set($cronReviewRequest.get() + 1) diff --git a/apps/desktop/src/store/onboarding.test.ts b/apps/desktop/src/store/onboarding.test.ts index 0a5fb1578e..d6b1f6698a 100644 --- a/apps/desktop/src/store/onboarding.test.ts +++ b/apps/desktop/src/store/onboarding.test.ts @@ -416,6 +416,64 @@ describe('OAuth onboarding', () => { expect(recommendedIndex).toBeGreaterThan(optionsIndex) expect(setIndex).toBeGreaterThan(recommendedIndex) }) + + it('does not advance when the default model assignment is not persisted', async () => { + const model = 'openai/gpt-5.5-pro' + installApiMock(async ({ path }: { path: string }) => { + if (path === '/api/providers/oauth/nous/submit') { + return { ok: true, status: 'approved' } + } + if (path.startsWith('/api/model/options')) { + return { providers: [{ name: 'Nous Portal', slug: 'nous', models: [model] }] } + } + if (path.startsWith('/api/model/recommended-default?')) { + return { provider: 'nous', model, free_tier: false } + } + if (path === '/api/model/set') { + return { + ok: false, + provider: 'nous', + model, + confirm_required: true, + confirm_message: 'Confirm this expensive model.' + } + } + + throw new Error(`unexpected api path: ${path}`) + }) + + const requestGatewayMock = vi.fn(async (method: string) => { + if (method === 'reload.env') { + return {} + } + + throw new Error(`unexpected gateway method: ${method}`) + }) + const requestGateway = requestGatewayMock as OnboardingContext['requestGateway'] + $desktopOnboarding.set( + baseState({ + flow: { + status: 'awaiting_user', + provider: provider('nous', 'Nous Portal'), + start: { + auth_url: 'https://portal.example/auth', + expires_in: 600, + flow: 'pkce', + session_id: 'portal-session' + }, + code: 'fresh-code' + }, + requested: true + }) + ) + + await submitOnboardingCode(onboardingContext(requestGateway)) + + const state = $desktopOnboarding.get() + expect(state.flow.status).toBe('error') + expect(state.flow.status === 'error' ? state.flow.message : '').toContain('Confirm this expensive model.') + expect(requestGatewayMock).not.toHaveBeenCalledWith('setup.runtime_check', expect.anything()) + }) }) describe('saveOnboardingLocalEndpoint', () => { diff --git a/apps/desktop/src/store/onboarding.ts b/apps/desktop/src/store/onboarding.ts index 83a81598b6..3043d3245e 100644 --- a/apps/desktop/src/store/onboarding.ts +++ b/apps/desktop/src/store/onboarding.ts @@ -7,13 +7,13 @@ import { listOAuthProviders, pollOAuthSession, setEnvVar, - setModelAssignment, startOAuthLogin, submitOAuthCode, validateProviderCredential } from '@/hermes' import { isProviderSetupErrorMessage } from '@/lib/provider-setup-errors' import { evaluateRuntimeReadiness, type RuntimeReadinessResult } from '@/lib/runtime-readiness' +import { setMainModelAssignment } from '@/store/cron-model-impact' import { notify, notifyError } from '@/store/notifications' import type { ModelOptionProvider, OAuthProvider, OAuthStartResponse } from '@/types/hermes' @@ -318,16 +318,16 @@ async function completeWithModelConfirm( // config provider (e.g. anthropic from a prior failed setup) cannot make // setup.runtime_check validate the wrong backend after a fresh OAuth login. try { - const res = await setModelAssignment({ - scope: 'main', + const res = await setMainModelAssignment({ provider: defaults.providerSlug, model: defaults.defaultModel }) notifyGatewayTools(res.gateway_tools) - } catch { - // Persistence failed — still run the scoped runtime check below and - // show the confirm card so the user can pick something explicitly. + } catch (error) { + onFail(error instanceof Error ? error.message : 'Hermes could not save the selected model.') + + return } } @@ -846,7 +846,7 @@ export async function saveOnboardingLocalEndpoint(baseUrl: string, apiKey: strin } try { - await setModelAssignment({ scope: 'main', provider: 'custom', model, base_url: url, api_key: key }) + await setMainModelAssignment({ provider: 'custom', model, base_url: url, api_key: key }) await ctx.requestGateway('reload.env').catch(() => undefined) const runtime = await checkRuntime(ctx) @@ -883,8 +883,7 @@ export async function setOnboardingModel(model: string) { setFlow({ ...flow, currentModel: model, saving: true }) try { - await setModelAssignment({ - scope: 'main', + await setMainModelAssignment({ provider: flow.providerSlug, model }) diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index c97614884d..0d38269479 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -11,6 +11,7 @@ import { storedStringArray, storedStringRecord } from '@/lib/storage' +import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' import { $gateway, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway' import { setConnection } from '@/store/session' import { resetStarmapGraph } from '@/store/starmap' @@ -176,6 +177,7 @@ $activeGatewayProfile.subscribe(value => { setApiRequestProfile(key) if (_lastRoutedProfile !== null && _lastRoutedProfile !== key) { + invalidateCronModelImpactScopeState() // Profile-scoped settings + the unified session list are now stale. // Narrowed so account/marketplace/onboarding caches don't refetch on // every profile switch. diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 497909a10c..b06fc5d243 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -6,6 +6,7 @@ import type { ContextSuggestion } from '@/app/types' import type { HermesConnection } from '@/global' import type { ChatMessage } from '@/lib/chat-messages' import { persistBoolean, persistString, storedBoolean, storedString } from '@/lib/storage' +import { syncCronModelImpactConnection } from '@/store/cron-model-impact-scope' import type { SessionInfo, UsageStats } from '@/types/hermes' type Updater = T | ((current: T) => T) @@ -621,7 +622,10 @@ export const $contextSuggestions = atom([]) export const $modelPickerOpen = atom(false) export const $sessionPickerOpen = atom(false) -export const setConnection = (next: Updater) => updateAtom($connection, next) +export const setConnection = (next: Updater) => { + updateAtom($connection, next) + syncCronModelImpactConnection($connection.get()) +} export const setGatewayState = (next: Updater) => updateAtom($gatewayState, next) export const setSessions = (next: Updater) => updateAtom($sessions, next) export const setCronSessions = (next: Updater) => updateAtom($cronSessions, next) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 26ccc6db6e..2afe4e44a7 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -1268,6 +1268,22 @@ export interface StaleAuxAssignment { model: string } +export type CronModelDriftAxis = 'model' | 'provider' + +export interface CronModelImpactJob { + id: string + name: string + drifted_axes: CronModelDriftAxis[] +} + +export interface CronModelImpact { + available: boolean + guard_enabled: boolean + affected_count: number + truncated: boolean + jobs: CronModelImpactJob[] +} + /** One skill-hub source (official index, GitHub, skills.sh, …) as reported by * `GET /api/skills/hub/sources`. */ export interface SkillHubSource { @@ -1423,6 +1439,10 @@ export interface ModelAssignmentResponse { * switching the main provider to Nous. Empty unless provider === 'nous' * and the user is a paid subscriber with unconfigured tools. */ gateway_tools?: string[] + /** Additive profile-local cron impact returned after a persisted main assignment. */ + cron_model_impact?: CronModelImpact + confirm_message?: string + confirm_required?: boolean model?: string ok: boolean provider?: string diff --git a/cron/scheduler.py b/cron/scheduler.py index 7c7d2b9c51..14cac7054f 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -45,8 +45,10 @@ from hermes_constants import get_hermes_home from hermes_cli._subprocess_compat import windows_hide_flags from hermes_cli.config import ( _expand_env_vars, + cron_model_drift_axes, cron_model_drift_guard_enabled, load_config, + resolve_cron_model_drift_defaults, ) from hermes_cli.fallback_config import get_fallback_chain from hermes_time import now as _hermes_now @@ -3790,14 +3792,12 @@ def run_job( # Cron-fleet default beats the global chat model: it is # the user's explicit "cron runs on this" setting. model = _cron_default_model - elif isinstance(_model_cfg, str): - model = _model_cfg - elif isinstance(_model_cfg, dict): - # Mirror the CLI/oneshot resolution: prefer ``default``, - # accept a ``model`` alias, overwrite only when truthy. - _default = _model_cfg.get("default") or _model_cfg.get("model") - if _default: - model = _default + else: + # Shared with Desktop's post-save impact summary so both + # paths compare snapshots against the same global model. + _, _global_model = resolve_cron_model_drift_defaults(_cfg) + if _global_model: + model = _global_model except Exception as e: logger.warning("Job '%s': failed to load config.yaml, using defaults: %s", job_id, e) @@ -4051,30 +4051,19 @@ def run_job( # unpinned cron jobs there, so the guard is skipped for that axis. if cron_model_drift_guard_enabled(_cfg): _drift: list[str] = [] - _provider_snapshot = (job.get("provider_snapshot") or "").strip().lower() - if ( - _provider_snapshot - and not (job.get("provider") or "").strip() - and not _cron_default_provider + _current_provider = str( + primary_provider_for_drift or runtime.get("provider") or "" + ).strip().lower() + _current_model = str(primary_model_for_drift or "").strip().lower() + for _axis in cron_model_drift_axes( + job, + current_provider=_current_provider, + current_model=_current_model, + config=_cfg, ): - _current_provider = str( - primary_provider_for_drift or runtime.get("provider") or "" - ).strip().lower() - if _current_provider and _current_provider != _provider_snapshot: - _drift.append( - f"provider '{_provider_snapshot}' -> '{_current_provider}'" - ) - _model_snapshot = (job.get("model_snapshot") or "").strip().lower() - if ( - _model_snapshot - and not (job.get("model") or "").strip() - and not _cron_default_model - ): - _current_model = str(primary_model_for_drift or "").strip().lower() - if _current_model and _current_model != _model_snapshot: - _drift.append( - f"model '{_model_snapshot}' -> '{_current_model}'" - ) + _snapshot = str(job.get(f"{_axis}_snapshot") or "").strip().lower() + _current = _current_provider if _axis == "provider" else _current_model + _drift.append(f"{_axis} '{_snapshot}' -> '{_current}'") if _drift: _changes = "; ".join(_drift) logger.warning( diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8395df4af5..c863e214f2 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -27,6 +27,7 @@ import sys import tempfile import threading import time +import unicodedata from dataclasses import dataclass from pathlib import Path from typing import Dict, Any, Optional, List, Tuple, Set @@ -4640,16 +4641,182 @@ def _cron_fleet_default_covers_axis( return isinstance(value, str) and bool(value.strip()) -def _load_cron_jobs_for_config_warning() -> List[Dict[str, Any]]: - """Best-effort read of the active profile's cron jobs database. +_CRON_MODEL_IMPACT_JOB_LIMIT = 50 +_CRON_MODEL_IMPACT_ID_LIMIT = 256 +_CRON_MODEL_IMPACT_NAME_LIMIT = 120 - Delegates to ``cron.jobs.load_jobs`` to reuse its BOM handling, corruption - repair, and context-local store resolution (tests, embedders). Falls back - to an empty list on any failure so config writes never break. + +def _model_assignment_text(value: Any) -> str: + """Return a trimmed scalar model/provider value, or empty for malformed data.""" + return value.strip() if isinstance(value, str) else "" + + +def resolve_cron_model_drift_defaults( + config: Any, + *, + environ: Optional[Dict[str, str]] = None, +) -> Tuple[str, str]: + """Resolve the global provider/model values cron compares against snapshots. + + Mirrors the scheduler's global-model precedence: a truthy configured model + wins ``HERMES_MODEL``; the environment is only a fallback. Per-job and cron + fleet defaults are handled by the caller/classifier because they suppress a + drift axis rather than changing the global assignment. """ + env = os.environ if environ is None else environ + provider = "" + model = _model_assignment_text(env.get("HERMES_MODEL", "")) + model_config = config.get("model") if isinstance(config, dict) else None + if isinstance(model_config, str): + configured_model = model_config.strip() + if configured_model: + model = configured_model + elif isinstance(model_config, dict): + provider = _model_assignment_text(model_config.get("provider")) + configured_model = _model_assignment_text( + model_config.get("default") + or model_config.get("model") + or model_config.get("name") + ) + if configured_model: + model = configured_model + return provider, model + + +def cron_model_drift_axes( + job: Any, + *, + current_provider: Any = "", + current_model: Any = "", + config: Any = None, +) -> List[str]: + """Return the unpinned axes that the fail-closed cron guard would block.""" + if not isinstance(job, dict) or not cron_model_drift_guard_enabled(config): + return [] + + current = { + "provider": _model_assignment_text(current_provider).lower(), + "model": _model_assignment_text(current_model).lower(), + } + drifted: List[str] = [] + for axis in ("provider", "model"): + if _cron_fleet_default_covers_axis(axis, config): + continue + if _model_assignment_text(job.get(axis)): + continue + snapshot = _model_assignment_text(job.get(f"{axis}_snapshot")).lower() + if snapshot and current[axis] and snapshot != current[axis]: + drifted.append(axis) + return drifted + + +def _valid_cron_impact_job_id(value: Any) -> str: + if not isinstance(value, str): + return "" + job_id = value.strip() + if not job_id or len(job_id) > _CRON_MODEL_IMPACT_ID_LIMIT: + return "" + if any(unicodedata.category(char).startswith("C") for char in job_id): + return "" + return job_id + + +def _cron_impact_job_name(value: Any, job_id: str) -> str: + if isinstance(value, str): + printable = "".join( + char for char in value if not unicodedata.category(char).startswith("C") + ) + name = " ".join(printable.split())[:_CRON_MODEL_IMPACT_NAME_LIMIT].rstrip() + if name: + return name + return f"Job {job_id}"[:_CRON_MODEL_IMPACT_NAME_LIMIT].rstrip() + + +def _unavailable_cron_model_impact(guard_enabled: bool) -> Dict[str, Any]: + return { + "available": False, + "guard_enabled": guard_enabled, + "affected_count": 0, + "truncated": False, + "jobs": [], + } + + +def build_cron_model_impact( + *, + current_provider: Any = "", + current_model: Any = "", + config: Any = None, + jobs: Any = None, +) -> Dict[str, Any]: + """Build a bounded, profile-local summary of jobs blocked by model drift. + + Job-store inspection is deliberately best effort: a model assignment has + already succeeded by the time Desktop requests this summary, so an unreadable + store is represented as unavailable instead of failing or rolling back it. + """ + guard_enabled = cron_model_drift_guard_enabled(config) + if jobs is None: + try: + from cron.jobs import load_jobs + + jobs = load_jobs() + except Exception: + return _unavailable_cron_model_impact(guard_enabled) + if not isinstance(jobs, list): + return _unavailable_cron_model_impact(guard_enabled) + + result: Dict[str, Any] = { + "available": True, + "guard_enabled": guard_enabled, + "affected_count": 0, + "truncated": False, + "jobs": [], + } + if not guard_enabled: + return result + + from cron.jobs import is_job_runnable + + seen_ids: Set[str] = set() + for job in jobs: + if not isinstance(job, dict): + continue + if not is_job_runnable(job) or job.get("no_agent"): + continue + job_id = _valid_cron_impact_job_id(job.get("id")) + if not job_id or job_id in seen_ids: + continue + seen_ids.add(job_id) + axes = cron_model_drift_axes( + job, + current_provider=current_provider, + current_model=current_model, + config=config, + ) + if not axes: + continue + result["affected_count"] += 1 + if len(result["jobs"]) < _CRON_MODEL_IMPACT_JOB_LIMIT: + result["jobs"].append( + { + "id": job_id, + "name": _cron_impact_job_name(job.get("name"), job_id), + "drifted_axes": axes, + } + ) + + result["truncated"] = result["affected_count"] > len(result["jobs"]) + return result + + +def _load_cron_jobs_for_config_warning() -> List[Dict[str, Any]]: + """Best-effort read of the active profile's cron jobs database.""" try: from cron.jobs import load_jobs - return load_jobs() + + jobs = load_jobs() + return jobs if isinstance(jobs, list) else [] except Exception: return [] @@ -4659,46 +4826,25 @@ def warn_unpinned_cron_jobs_after_model_config_change( value: Any, config: Optional[Dict[str, Any]] = None, ) -> None: - """Warn when a global model/provider change will trip cron's drift guard. - - Cron intentionally fails closed when an unpinned agent job's current global - model/provider differs from its creation-time snapshot. Surface that outcome - when the operator changes the global axis instead of letting the next tick - be the first visible signal. - """ + """Warn when a global model/provider change will trip cron's drift guard.""" axis = _cron_model_drift_axis_for_config_key(key) if axis is None: return - if not cron_model_drift_guard_enabled(config): - return - # A cron-fleet default covering this axis (cron.model / - # cron.model_provider) means unpinned jobs no longer follow the global - # value at all — the drift guard will not engage, so warning here would - # be a false alarm. - if _cron_fleet_default_covers_axis(axis, config): - return - new_value = str(value or "").strip().lower() + new_value = _model_assignment_text(value) if not new_value: return - - pinned_field = axis - snapshot_field = f"{axis}_snapshot" - affected = 0 - for job in _load_cron_jobs_for_config_warning(): - if not job.get("enabled", True): - continue - if job.get("no_agent"): - continue - if str(job.get(pinned_field) or "").strip(): - continue - snapshot = str(job.get(snapshot_field) or "").strip().lower() - if snapshot and snapshot != new_value: - affected += 1 - + impact = build_cron_model_impact( + current_provider=new_value if axis == "provider" else "", + current_model=new_value if axis == "model" else "", + config=config, + jobs=_load_cron_jobs_for_config_warning(), + ) + affected = impact["affected_count"] if affected <= 0: return + snapshot_field = f"{axis}_snapshot" noun = "job" if affected == 1 else "jobs" verb = "has" if affected == 1 else "have" print( diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 5e378bdda7..f38ce4c3d9 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -58,6 +58,7 @@ if str(PROJECT_ROOT) not in sys.path: from hermes_cli import __version__, __release_date__ from hermes_cli.config import ( + build_cron_model_impact, cfg_get, DEFAULT_CONFIG, OPTIONAL_ENV_VARS, @@ -69,6 +70,7 @@ from hermes_cli.config import ( load_config, load_env, read_raw_config, + resolve_cron_model_drift_defaults, save_config, save_env_value, remove_env_value, @@ -6804,6 +6806,20 @@ def _apply_model_assignment_sync( "model": str(slot_cfg.get("model", "") or ""), }) + try: + effective_config = load_config() + effective_provider, effective_model = resolve_cron_model_drift_defaults( + effective_config + ) + cron_model_impact = build_cron_model_impact( + current_provider=effective_provider or provider, + current_model=effective_model or model, + config=effective_config, + ) + except Exception: + _log.debug("cron model impact inspection failed", exc_info=True) + cron_model_impact = build_cron_model_impact(config=cfg, jobs={}) + return { "ok": True, "scope": "main", @@ -6812,6 +6828,7 @@ def _apply_model_assignment_sync( "base_url": model_cfg.get("base_url", ""), "gateway_tools": gateway_tools, "stale_aux": stale_aux, + "cron_model_impact": cron_model_impact, } # scope == "auxiliary" diff --git a/tests/hermes_cli/test_cron_model_impact.py b/tests/hermes_cli/test_cron_model_impact.py new file mode 100644 index 0000000000..933c8d2dce --- /dev/null +++ b/tests/hermes_cli/test_cron_model_impact.py @@ -0,0 +1,201 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from hermes_cli.config import ( + build_cron_model_impact, + cron_model_drift_axes, + resolve_cron_model_drift_defaults, +) + + +def _job(**overrides: Any) -> dict[str, Any]: + job = { + "id": "job-1", + "name": "Morning summary", + "enabled": True, + "no_agent": False, + "provider_snapshot": "openrouter", + "model_snapshot": "old/model", + } + job.update(overrides) + return job + + +def _impact(jobs: object, **config: Any) -> dict[str, Any]: + return build_cron_model_impact( + current_provider="nous", + current_model="new/model", + config=config, + jobs=jobs, + ) + + +def test_drift_axes_match_unpinned_guard_semantics() -> None: + assert cron_model_drift_axes( + _job(), current_provider=" NOUS ", current_model="NEW/MODEL", config={} + ) == ["provider", "model"] + assert cron_model_drift_axes( + _job(provider="openrouter"), + current_provider="nous", + current_model="new/model", + config={}, + ) == ["model"] + assert cron_model_drift_axes( + _job(model="old/model", provider="openrouter"), + current_provider="nous", + current_model="new/model", + config={}, + ) == [] + + +def test_fleet_defaults_and_literal_false_guard_suppress_only_intended_axes() -> None: + assert cron_model_drift_axes( + _job(), + current_provider="nous", + current_model="new/model", + config={"cron": {"model": "fleet/model"}}, + ) == ["provider"] + assert cron_model_drift_axes( + _job(), + current_provider="nous", + current_model="new/model", + config={"cron": {"model_provider": "openrouter"}}, + ) == ["model"] + assert cron_model_drift_axes( + _job(), + current_provider="nous", + current_model="new/model", + config={"cron": {"model_drift_guard": False}}, + ) == [] + assert cron_model_drift_axes( + _job(), + current_provider="nous", + current_model="new/model", + config={"cron": {"model_drift_guard": "false"}}, + ) == ["provider", "model"] + + +def test_summary_filters_disabled_no_agent_and_mixed_pins() -> None: + jobs = [ + _job(id="disabled", enabled=False), + _job(id="script", no_agent=True), + _job(id="paused-state", state="paused"), + _job(id="paused-marker", paused_at="2026-08-11T00:00:00Z"), + _job(id="fully-pinned", model="old/model", provider="openrouter"), + _job(id="model-pinned", model="old/model"), + _job(id="provider-pinned", provider="openrouter"), + ] + + impact = _impact(jobs) + + assert impact == { + "available": True, + "guard_enabled": True, + "affected_count": 2, + "truncated": False, + "jobs": [ + {"id": "model-pinned", "name": "Morning summary", "drifted_axes": ["provider"]}, + {"id": "provider-pinned", "name": "Morning summary", "drifted_axes": ["model"]}, + ], + } + + +def test_summary_handles_legacy_and_malformed_records_without_hiding_valid_siblings() -> None: + impact = _impact( + [ + None, + "bad", + {"id": ["not-a-string"]}, + _job(id="job-1", name=" Morning\u0000 summary "), + _job(id="job-1", name="duplicate"), + _job(id="job-2", name=42), + _job(id="missing-snapshot", provider_snapshot=None, model_snapshot={}), + ] + ) + + assert impact["affected_count"] == 2 + assert impact["jobs"] == [ + {"id": "job-1", "name": "Morning summary", "drifted_axes": ["provider", "model"]}, + {"id": "job-2", "name": "Job job-2", "drifted_axes": ["provider", "model"]}, + ] + + +@pytest.mark.parametrize("jobs", [{"jobs": []}, "jobs", 7]) +def test_non_list_job_collection_is_unavailable(jobs: object) -> None: + assert _impact(jobs) == { + "available": False, + "guard_enabled": True, + "affected_count": 0, + "truncated": False, + "jobs": [], + } + + +def test_summary_bounds_id_name_and_payload() -> None: + valid = [_job(id=f"job-{index:03}", name=f" Job {index} ") for index in range(51)] + jobs = [ + _job(id="x" * 257), + _job(id="bad\u200bid"), + _job(id="edge", name="x" * 121), + *valid, + ] + + impact = _impact(jobs) + + assert impact["affected_count"] == 52 + assert len(impact["jobs"]) == 50 + assert impact["truncated"] is True + assert impact["jobs"][0]["id"] == "edge" + assert impact["jobs"][0]["name"] == "x" * 120 + assert impact["jobs"][-1]["id"] == "job-048" + + +def test_fallback_name_respects_the_desktop_code_point_limit() -> None: + job_id = "x" * 256 + + impact = _impact([_job(id=job_id, name=42)]) + + assert impact["affected_count"] == 1 + assert impact["jobs"][0]["name"] == (f"Job {job_id}")[:120] + + +def test_guard_disabled_summary_is_available_but_empty() -> None: + assert _impact([_job()], cron={"model_drift_guard": False}) == { + "available": True, + "guard_enabled": False, + "affected_count": 0, + "truncated": False, + "jobs": [], + } + + +def test_effective_defaults_match_scheduler_config_over_env_precedence() -> None: + assert resolve_cron_model_drift_defaults( + {"model": {"provider": "managed", "default": "managed/model"}}, + environ={"HERMES_MODEL": "env/model"}, + ) == ("managed", "managed/model") + assert resolve_cron_model_drift_defaults( + {"model": {}}, environ={"HERMES_MODEL": "env/model"} + ) == ("", "env/model") + assert resolve_cron_model_drift_defaults( + {"model": "legacy/model"}, environ={"HERMES_MODEL": "env/model"} + ) == ("", "legacy/model") + + +def test_loader_exception_returns_unavailable(monkeypatch: pytest.MonkeyPatch) -> None: + import cron.jobs + + def fail() -> list[dict[str, Any]]: + raise RuntimeError("broken store") + + monkeypatch.setattr(cron.jobs, "load_jobs", fail) + + impact = build_cron_model_impact( + current_provider="nous", current_model="new/model", config={} + ) + + assert impact["available"] is False + assert impact["affected_count"] == 0 diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index a0534ac273..b1d60df0d9 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -6,6 +6,8 @@ profile switcher can target any profile's HERMES_HOME. These tests pin: reads/writes land in the REQUESTED profile, the dashboard's own profile stays untouched, and the chat PTY env is scoped via HERMES_HOME. """ +import json + import pytest import yaml @@ -50,6 +52,12 @@ def _cfg(home): return yaml.safe_load((home / "config.yaml").read_text()) or {} +def _write_jobs(home, jobs): + cron_dir = home / "cron" + cron_dir.mkdir(parents=True, exist_ok=True) + (cron_dir / "jobs.json").write_text(json.dumps(jobs), encoding="utf-8") + + class TestProfileScopedConfig: @@ -180,6 +188,113 @@ class TestProfileScopedModel: if isinstance(default_model, dict): assert default_model.get("default") != "test/model-1" + def test_main_assignment_reports_only_target_profile_cron_impact( + self, client, isolated_profiles + ): + stale = { + "name": "Worker summary", + "enabled": True, + "no_agent": False, + "provider_snapshot": "openrouter", + "model_snapshot": "old/model", + } + _write_jobs( + isolated_profiles["worker_beta"], [{"id": "worker-job", **stale}] + ) + _write_jobs( + isolated_profiles["default"], + [{"id": "default-job", **stale, "name": "Default summary"}], + ) + + resp = client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "nous", + "model": "new/model", + "confirm_expensive_model": True, + "profile": "worker_beta", + }, + ) + + assert resp.status_code == 200 + assert resp.json()["cron_model_impact"] == { + "available": True, + "guard_enabled": True, + "affected_count": 1, + "truncated": False, + "jobs": [ + { + "id": "worker-job", + "name": "Worker summary", + "drifted_axes": ["provider", "model"], + } + ], + } + + def test_unavailable_impact_does_not_fail_persisted_assignment( + self, client, isolated_profiles, monkeypatch + ): + import cron.jobs + + monkeypatch.setattr(cron.jobs, "load_jobs", lambda: {"malformed": True}) + + resp = client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "nous", + "model": "new/model", + "confirm_expensive_model": True, + "profile": "worker_beta", + }, + ) + + assert resp.status_code == 200 + assert resp.json()["ok"] is True + assert resp.json()["cron_model_impact"]["available"] is False + assert _cfg(isolated_profiles["worker_beta"])["model"]["default"] == "new/model" + + def test_auxiliary_and_confirmation_responses_have_no_impact_summary( + self, client, isolated_profiles + ): + _write_jobs( + isolated_profiles["worker_beta"], + [ + { + "id": "worker-job", + "enabled": True, + "provider_snapshot": "openrouter", + "model_snapshot": "old/model", + } + ], + ) + + auxiliary = client.post( + "/api/model/set", + json={ + "scope": "auxiliary", + "provider": "nous", + "model": "new/model", + "profile": "worker_beta", + }, + ) + confirmation = client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "openrouter", + "model": "openai/gpt-5.5-pro", + "profile": "worker_beta", + }, + ) + + assert auxiliary.status_code == 200 + assert "cron_model_impact" not in auxiliary.json() + assert confirmation.status_code == 200 + assert confirmation.json()["confirm_required"] is True + assert "cron_model_impact" not in confirmation.json() +