From dda88ecbac522aa3cc899c6b4c7a15a571e67966 Mon Sep 17 00:00:00 2001 From: BearHuddleston Date: Sun, 20 Sep 2026 04:31:33 -0500 Subject: [PATCH 01/15] fix(desktop): keep steering recovery bound to its source chat --- .../session/hooks/use-prompt-actions/index.ts | 71 ++- .../steering-recovery.test.tsx | 427 ++++++++++++++++++ .../use-prompt-actions/steering-session.ts | 85 ++++ 3 files changed, 560 insertions(+), 23 deletions(-) create mode 100644 apps/desktop/src/app/session/hooks/use-prompt-actions/steering-recovery.test.tsx create mode 100644 apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index beefd9b04c..253b1a28ec 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -68,6 +68,7 @@ import { type SurvivorUserRowIds } from './rewind' import { useSlashCommand } from './slash' +import { captureSteeringSession } from './steering-session' import { useSubmitPrompt } from './submit' import { blobToDataUrl, @@ -746,12 +747,20 @@ export function usePromptActions({ const redirectPrompt = useCallback( async (rawText: string): Promise => { const text = sanitizeComposerInput(rawText).trim() + // Ref, not the closure-captured prop — see cancelRun above. A redirect // reaches the live model mid-turn, so a stale target delivers the user's // correction into a conversation they are no longer looking at. - const sessionId = activeSessionIdRef.current + const target = captureSteeringSession({ + activeSessionIdRef, + selectedStoredSessionIdRef, + runtimeIdByStoredSessionIdRef, + getRoutedStoredSessionId, + requestGateway, + updateSessionState + }) - if (!text || !sessionId) { + if (!text || !target) { return false } @@ -766,7 +775,9 @@ export function usePromptActions({ // gateway, in arrival order: sealed already-streamed output above, // correction bubble below it, post-redirect deltas below that // (#73793, #83151). - const messageId = appendSessionTextMessage(id, 'user', text, undefined, { appendAfterActiveReply: true }) + const messageId = appendSessionTextMessage(id, 'user', text, target.storedSessionId, { + appendAfterActiveReply: true + }) const discardOptimisticMessage = () => updateSessionState(id, state => ({ @@ -784,7 +795,10 @@ export function usePromptActions({ }) try { - const result = await requestGateway('session.redirect', { session_id: id, text }) + const result = await target.requestGateway('session.redirect', { + session_id: id, + text + }) if (result?.status === 'redirected') { triggerHaptic('submit') @@ -814,13 +828,7 @@ export function usePromptActions({ // A stale runtime id after reconnect 404s ("session not found"): the // shared resolver resumes the stored session and retries once, so a // correction right after a reconnect isn't lost to the race. - const { result } = await withSessionNotFoundResume(sessionId, selectedStoredSessionIdRef.current, send, { - requestGateway, - onRecovered: recoveredId => { - activeSessionIdRef.current = recoveredId - setActiveSessionId(recoveredId) - } - }) + const { result } = await withSessionNotFoundResume(target.sessionId, target.storedSessionId, send, target) return result } catch { @@ -829,7 +837,15 @@ export function usePromptActions({ return false }, - [activeSessionIdRef, appendSessionTextMessage, requestGateway, selectedStoredSessionIdRef, updateSessionState] + [ + activeSessionIdRef, + appendSessionTextMessage, + getRoutedStoredSessionId, + requestGateway, + runtimeIdByStoredSessionIdRef, + selectedStoredSessionIdRef, + updateSessionState + ] ) // A hidden note that lands mid-turn must reach the model without becoming a @@ -839,33 +855,42 @@ export function usePromptActions({ const injectHiddenPrompt = useCallback( async (rawText: string): Promise => { const text = sanitizeComposerInput(rawText).trim() - const sessionId = activeSessionIdRef.current - if (!text || !sessionId) { + const target = captureSteeringSession({ + activeSessionIdRef, + selectedStoredSessionIdRef, + runtimeIdByStoredSessionIdRef, + getRoutedStoredSessionId, + requestGateway, + updateSessionState + }) + + if (!text || !target) { return false } const send = async (id: string): Promise => { - const response = await requestGateway('session.steer', { session_id: id, text }) + const response = await target.requestGateway('session.steer', { session_id: id, text }) return response?.status === 'queued' } try { - const { result } = await withSessionNotFoundResume(sessionId, selectedStoredSessionIdRef.current, send, { - requestGateway, - onRecovered: recoveredId => { - activeSessionIdRef.current = recoveredId - setActiveSessionId(recoveredId) - } - }) + const { result } = await withSessionNotFoundResume(target.sessionId, target.storedSessionId, send, target) return result } catch { return false } }, - [activeSessionIdRef, requestGateway, selectedStoredSessionIdRef] + [ + activeSessionIdRef, + getRoutedStoredSessionId, + requestGateway, + runtimeIdByStoredSessionIdRef, + selectedStoredSessionIdRef, + updateSessionState + ] ) // After a durable rewind the surviving bubbles' cached rowIds are stale (the diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-recovery.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-recovery.test.tsx new file mode 100644 index 0000000000..8f7e444bb3 --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-recovery.test.tsx @@ -0,0 +1,427 @@ +import { useStore } from '@nanostores/react' +import { QueryClient } from '@tanstack/react-query' +import { act, cleanup, render, waitFor } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' + +import { createSessionRpcDispatcher } from '@/app/contrib/session-rpc-dispatcher' +import { sessionRoute } from '@/app/routes' +import { textPart } from '@/lib/chat-messages' +import { requestGatewayForAgent, requestGatewayForProfile } from '@/store/gateway' +import { + $activeSessionId, + $activeSessionStoredIdRotation, + $messages, + $selectedStoredSessionId, + setActiveSessionId, + setAwaitingResponse, + setBusy, + setMessages, + setSelectedStoredSessionId, + setSessions +} from '@/store/session' +import { clearAllSessionStates } from '@/store/session-states' +import type { SessionInfo } from '@/types/hermes' + +import { handleSessionInfoEvent } from '../use-message-stream/gateway-event/session-info' +import { useSessionStateCache } from '../use-session-state-cache' + +import { clearSingleFlightSessionResumeState, registerRecoveredRuntime } from './single-flight-resume' + +import { usePromptActions } from '.' + +// Real prompt hooks, cache, ownership router, and dispatcher; only the external +// gateway edge is substituted. No test injects a corrupted ownership mapping. +vi.mock('@/store/gateway', async original => ({ + ...(await original>()), + requestGatewayForAgent: vi.fn(), + requestGatewayForProfile: vi.fn(), + retainGatewayForSessionTurn: vi.fn(async () => () => undefined) +})) + +const busyRef = { current: false } +let routedStoredId: string | null = 'stored-B' +let handle: { actions: ReturnType; cache: ReturnType } + +function Harness() { + const activeSessionId = useStore($activeSessionId) + const selectedStoredSessionId = useStore($selectedStoredSessionId) + + const cache = useSessionStateCache({ + activeSessionId, + selectedStoredSessionId, + busyRef, + setAwaitingResponse, + setBusy, + setMessages + }) + + const requestGateway = createSessionRpcDispatcher({ + ...cache, + ambientRequest: async () => { + throw new Error('unexpected ambient request') + } + }) + + const actions = usePromptActions({ + activeSessionId, + ...cache, + busyRef, + branchCurrentSession: async () => false, + createBackendSessionForSend: async () => { + throw new Error('unexpected create') + }, + getRoutedStoredSessionId: () => routedStoredId, + getRouteToken: () => `${routedStoredId ? sessionRoute(routedStoredId) : '/'}::`, + handleSkinCommand: () => '', + openMemoryGraph: () => undefined, + refreshSessions: async () => undefined, + requestGateway, + resumeStoredSession: async () => { + throw new Error('unexpected foreground resume') + }, + startFreshSessionDraft: () => undefined, + sttEnabled: false + }) + + handle = { actions, cache } + + return null +} + +function seed() { + routedStoredId = 'stored-B' + busyRef.current = false + // Same profile name on distinct backends: profile equality is not ownership. + setSessions( + ['A', 'B'].map(id => ({ + id: `stored-${id}`, + connection_id: `connection-${id}`, + profile: 'default', + source: 'desktop', + message_count: 1 + })) as SessionInfo[] + ) + setSelectedStoredSessionId('stored-B') + setActiveSessionId('rt-B') + render() + act(() => { + for (const id of ['A', 'B']) { + handle.cache.updateSessionState( + `rt-${id}`, + state => ({ + ...state, + messages: [{ id: `history-${id}`, role: 'assistant', parts: [textPart(`history ${id}`)] }] + }), + `stored-${id}` + ) + } + }) +} + +function navigateToA() { + act(() => { + routedStoredId = 'stored-A' + setSelectedStoredSessionId('stored-A') + setActiveSessionId('rt-A') + }) + act(() => { + handle.cache.syncSessionStateToView('rt-A', handle.cache.sessionStateByRuntimeIdRef.current.get('rt-A')!) + }) +} + +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + + const promise = new Promise((yes, no) => { + resolve = yes + reject = no + }) + + return { promise, resolve, reject } +} + +afterEach(() => { + cleanup() + clearAllSessionStates() + clearSingleFlightSessionResumeState() + setActiveSessionId(null) + setSelectedStoredSessionId(null) + setSessions([]) + setBusy(false) + setAwaitingResponse(false) + setMessages([]) + window.localStorage.clear() + vi.clearAllMocks() +}) + +it.each(['redirectPrompt', 'injectHiddenPrompt'] as const)( + '%s refuses a source whose route, selection, and runtime do not agree', + async action => { + vi.mocked(requestGatewayForAgent).mockResolvedValue({ status: 'queued' }) + seed() + const originalB = handle.cache.sessionStateByRuntimeIdRef.current.get('rt-B') + // Route publication can precede both selection and runtime publication. + routedStoredId = 'stored-A' + await act(async () => { + expect(await handle.actions[action]('not B input')).toBe(false) + }) + expect(requestGatewayForAgent).not.toHaveBeenCalled() + expect(handle.cache.sessionStateByRuntimeIdRef.current.get('rt-B')).toBe(originalB) + + // Selection/route can also agree while the active runtime still lags. + act(() => { + setSelectedStoredSessionId('stored-A') + }) + // Missing forward proof must not turn a known foreign runtime into A's. + handle.cache.runtimeIdByStoredSessionIdRef.current.delete('stored-A') + await act(async () => { + expect(await handle.actions[action]('A input')).toBe(false) + }) + expect(requestGatewayForAgent).not.toHaveBeenCalled() + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.has('stored-A')).toBe(false) + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.get('stored-B')).toBe('rt-B') + + // A fresh draft must not steer the previous chat's still-live runtime. + act(() => { + routedStoredId = null + setSelectedStoredSessionId(null) + }) + await act(async () => { + expect(await handle.actions[action]('new chat input')).toBe(false) + }) + expect(requestGatewayForAgent).not.toHaveBeenCalled() + } +) + +const steeringActions = [ + { action: 'redirectPrompt' as const, method: 'session.redirect', status: 'redirected' }, + { action: 'redirectPrompt' as const, method: 'session.redirect', status: 'queued' }, + { action: 'injectHiddenPrompt' as const, method: 'session.steer', status: 'queued' } +] + +const recoveryCases = (['before-stale-error', 'during-resume', 'stay'] as const).flatMap(navigation => + [false, true].flatMap(expiredRecovery => steeringActions.map(action => ({ ...action, navigation, expiredRecovery }))) +) + +it.each(recoveryCases)( + '$action recovery ($status, $navigation, expired cache: $expiredRecovery) retains its owner without stealing another chat', + async ({ action, method, status, navigation, expiredRecovery }) => { + const first = deferred() + const resume = deferred<{ session_id: string }>() + vi.mocked(requestGatewayForAgent).mockImplementation(async (_connection, _profile, rpc, params) => { + if (rpc === method && params?.session_id === 'rt-B') { + return first.promise + } + + if (rpc === 'session.resume') { + return resume.promise + } + + if (rpc === method && params?.session_id === 'rt-B-cached') { + throw new Error('session not found') + } + + if (rpc === method) { + return { status } + } + + if (rpc === 'prompt.submit') { + return { status: 'streaming' } + } + + throw new Error(`unexpected ${rpc}`) + }) + seed() + + if (expiredRecovery) { + registerRecoveredRuntime('stored-B', 'rt-B-cached') + } + + let pending!: Promise + act(() => { + pending = handle.actions[action]('B correction') + }) + await waitFor(() => + expect(requestGatewayForAgent).toHaveBeenCalledWith('connection-B', 'default', method, { + session_id: 'rt-B', + text: 'B correction' + }) + ) + + if (navigation === 'before-stale-error') { + navigateToA() + } + + await act(async () => { + first.reject(new Error('session not found')) + }) + await waitFor(() => + expect(requestGatewayForAgent).toHaveBeenCalledWith( + 'connection-B', + 'default', + 'session.resume', + expect.objectContaining({ session_id: 'stored-B' }) + ) + ) + + if (expiredRecovery) { + expect(requestGatewayForAgent).toHaveBeenCalledWith('connection-B', 'default', method, { + session_id: 'rt-B-cached', + text: 'B correction' + }) + expect($activeSessionId.get()).toBe(navigation === 'before-stale-error' ? 'rt-A' : 'rt-B-cached') + } + + if (navigation === 'during-resume') { + navigateToA() + } + + await act(async () => { + resume.resolve({ session_id: 'rt-B2' }) + expect(await pending).toBe(true) + }) + + expect(requestGatewayForAgent).toHaveBeenCalledWith('connection-B', 'default', method, { + session_id: 'rt-B2', + text: 'B correction' + }) + expect(requestGatewayForProfile).not.toHaveBeenCalled() + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.get('stored-A')).toBe('rt-A') + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.get('stored-B')).toBe('rt-B2') + expect(handle.cache.sessionStateByRuntimeIdRef.current.get('rt-B2')?.storedSessionId).toBe('stored-B') + + const correctionRows = [...handle.cache.sessionStateByRuntimeIdRef.current.values()] + .flatMap(state => state.messages) + .filter(message => message.role === 'user') + + expect(correctionRows).toHaveLength(action === 'redirectPrompt' ? 1 : 0) + expect($activeSessionId.get()).toBe(navigation === 'stay' ? 'rt-B2' : 'rt-A') + expect(handle.cache.activeSessionIdRef.current).toBe($activeSessionId.get()) + expect($selectedStoredSessionId.get()).toBe(navigation === 'stay' ? 'stored-B' : 'stored-A') + + if (navigation === 'stay') { + navigateToA() + } + + expect($messages.get().map(message => message.id)).toEqual(['history-A']) + // An ordinary foreground send must still route to A after B's recovery. + await act(async () => { + expect(await handle.actions.submitText('A next prompt', { attachments: [], composerScope: 'stored-A' })).toBe( + true + ) + }) + expect(requestGatewayForAgent).toHaveBeenLastCalledWith( + 'connection-A', + 'default', + 'prompt.submit', + expect.objectContaining({ session_id: 'rt-A', text: 'A next prompt' }), + expect.any(Number), + undefined + ) + } +) + +const rebuiltRuntimeCases = steeringActions.flatMap(action => + [false, true].flatMap(recover => ['stored-B', 'stored-B-tip'].map(selection => ({ ...action, recover, selection }))) +) + +it.each(rebuiltRuntimeCases)( + '$action ($status, recovery: $recover, selection: $selection) preserves a rebuilt runtime’s tip binding and durable route', + async ({ action, method, status, recover, selection }) => { + seed() + const rotation = $activeSessionStoredIdRotation.get() + + act(() => { + setSessions([ + { + id: 'stored-B-tip', + _lineage_root_id: 'stored-B', + profile: 'default', + connection_id: 'connection-B', + source: 'desktop', + message_count: 2 + } + ] as SessionInfo[]) + handleSessionInfoEvent({ + deps: { + ...handle.cache, + activeGatewayProfile: 'default', + compactedTurnRef: { current: new Set() }, + lastCwdInfoSessionRef: { current: null }, + nativeSubagentSessionsRef: { current: new Set() }, + appendAssistantDelta: vi.fn(), + appendReasoningDelta: vi.fn(), + completeAssistantMessage: vi.fn(), + failAssistantMessage: vi.fn(), + flushQueuedDeltas: vi.fn(), + finalizeInterimAssistantMessage: vi.fn(), + hydrateFromStoredSession: vi.fn(async () => undefined), + queryClient: new QueryClient(), + refreshHermesConfig: vi.fn(async () => undefined), + scheduleSessionsRefresh: vi.fn(), + sessionInterrupted: () => false, + upsertToolCall: vi.fn() + }, + event: { profile: 'default', session_id: 'rt-B-rebuilt', type: 'session.info' }, + explicitSid: 'rt-B-rebuilt', + fromActiveSource: () => true, + isActiveEvent: false, + occurredAt: Date.now() / 1000, + payload: { stored_session_id: 'stored-B-tip', model: 'fixture-model', running: true }, + scheduleConfigRefresh: vi.fn(), + sessionId: 'rt-B-rebuilt' + }) + }) + // The real reducer adopts this runtime without rotating the durable selection. + expect($activeSessionId.get()).toBe('rt-B-rebuilt') + expect($selectedStoredSessionId.get()).toBe('stored-B') + expect(handle.cache.sessionStateByRuntimeIdRef.current.get('rt-B-rebuilt')?.storedSessionId).toBe('stored-B-tip') + + // A selection may also follow the tip while the durable route stays on root. + act(() => { + setSelectedStoredSessionId(selection) + }) + + vi.mocked(requestGatewayForAgent).mockImplementation(async (_connection, _profile, rpc, params) => { + if (rpc === 'session.resume') { + return { session_id: 'rt-B2' } + } + + if (recover && params?.session_id === 'rt-B-rebuilt') { + throw new Error('session not found') + } + + return { status } + }) + await act(async () => { + expect(await handle.actions[action]('same chat correction')).toBe(true) + }) + + const runtimeId = recover ? 'rt-B2' : 'rt-B-rebuilt' + expect(requestGatewayForAgent).toHaveBeenLastCalledWith('connection-B', 'default', method, { + session_id: runtimeId, + text: 'same chat correction' + }) + + if (recover) { + expect(requestGatewayForAgent).toHaveBeenCalledWith( + 'connection-B', + 'default', + 'session.resume', + expect.objectContaining({ session_id: 'stored-B-tip' }) + ) + } + + expect($activeSessionId.get()).toBe(runtimeId) + expect(handle.cache.activeSessionIdRef.current).toBe(runtimeId) + expect($selectedStoredSessionId.get()).toBe(selection) + expect(routedStoredId).toBe('stored-B') + expect(handle.cache.sessionStateByRuntimeIdRef.current.get('rt-B-rebuilt')?.storedSessionId).toBe('stored-B-tip') + expect(handle.cache.sessionStateByRuntimeIdRef.current.get(runtimeId)?.storedSessionId).toBe('stored-B-tip') + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.get('stored-B-tip')).toBe(runtimeId) + expect(handle.cache.runtimeIdByStoredSessionIdRef.current.get('stored-B')).toBe('rt-B') + expect($activeSessionStoredIdRotation.get()).toBe(rotation) + expect(requestGatewayForProfile).not.toHaveBeenCalled() + } +) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts new file mode 100644 index 0000000000..4c40e817fb --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts @@ -0,0 +1,85 @@ +import type { MutableRefObject } from 'react' + +import { findStoredIdForRuntimeId } from '@/app/contrib/wiring-routing' +import type { ClientSessionState } from '@/app/types' +import { $sessions, idsShareLineage, setActiveSessionId } from '@/store/session' +import { requestForSessionProfile } from '@/store/session-request-router' +import { knownOwnerForSession } from '@/store/session-states' + +import type { GatewayRequest } from './utils' + +interface SteeringSessionDeps { + activeSessionIdRef: MutableRefObject + selectedStoredSessionIdRef: MutableRefObject + runtimeIdByStoredSessionIdRef: MutableRefObject> + getRoutedStoredSessionId: () => string | null + requestGateway: GatewayRequest + updateSessionState: ( + sessionId: string, + updater: (state: ClientSessionState) => ClientSessionState, + storedSessionId?: string | null + ) => ClientSessionState +} + +/** Pin a correction to its source, not the selection at retry time. */ +export function captureSteeringSession(deps: SteeringSessionDeps) { + const { activeSessionIdRef, selectedStoredSessionIdRef, runtimeIdByStoredSessionIdRef, getRoutedStoredSessionId } = + deps + + const sessionId = activeSessionIdRef.current + const selectedStoredSessionId = selectedStoredSessionIdRef.current + const routedStoredSessionId = getRoutedStoredSessionId() + const bindings = runtimeIdByStoredSessionIdRef.current + const boundStoredSessionId = sessionId ? findStoredIdForRuntimeId(bindings, sessionId) : undefined + const sessions = $sessions.get() + + const matchesSelection = (id: string) => + Boolean(selectedStoredSessionId && idsShareLineage(id, selectedStoredSessionId, sessions)) + + // Navigation publishes these identities independently. Unlike ordinary Send, + // a mid-turn correction must not resolve another target and interrupt it. + if ( + !sessionId || + (routedStoredSessionId && !matchesSelection(routedStoredSessionId)) || + (selectedStoredSessionId && + selectedStoredSessionId !== sessionId && + (!boundStoredSessionId || !matchesSelection(boundStoredSessionId))) || + [...bindings].some(([stored, runtime]) => runtime === sessionId && !matchesSelection(stored)) + ) { + return null + } + + // A rebuilt runtime can bind the tip while selection/route keep the root. + // Use its proven stored id for writes; reapplying the root would rotate it back. + const storedSessionId = boundStoredSessionId ?? selectedStoredSessionId + const owner = knownOwnerForSession(sessionId) ?? knownOwnerForSession(storedSessionId) + let adoptedSessionId = sessionId + + const requestGateway: GatewayRequest = (method, params, timeoutMs) => + requestForSessionProfile(owner, deps.requestGateway, method, params, timeoutMs) + + return { + sessionId, + storedSessionId, + requestGateway, + resolveProfile: owner ? async () => (typeof owner === 'string' ? owner : owner.profile) : undefined, + onRecovered: (recoveredId: string) => { + // Both visible redirect and hidden steer need the binding BEFORE retry: + // hidden steer has no optimistic row to establish it incidentally. + deps.updateSessionState(recoveredId, state => state, storedSessionId) + + // The correction still belongs to its source when navigation backgrounds + // it. Keep its recovery, but only the unchanged view may adopt its id. + // A cached recovery can itself expire: the resolver then calls us again. + if ( + activeSessionIdRef.current === adoptedSessionId && + selectedStoredSessionIdRef.current === selectedStoredSessionId && + getRoutedStoredSessionId() === routedStoredSessionId + ) { + adoptedSessionId = recoveredId + activeSessionIdRef.current = recoveredId + setActiveSessionId(recoveredId) + } + } + } +} From 867f0cca2229594233d52bc63e3f38cdb7db855b Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:07:57 +0530 Subject: [PATCH 02/15] refactor(desktop): name the steering refusal predicates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four-clause guard in captureSteeringSession read as one boolean; split it into routeLeftSelection / runtimeUnbound / runtimeBoundElsewhere with the WHY per predicate, and return early on a null runtime instead of threading the null through every clause. Behaviour-preserving: the dropped disjunct of the old third clause (`!matchesSelection(boundStoredSessionId)`) was subsumed by the reverse-binding scan — `boundStoredSessionId` is itself one of the `[stored, runtime]` pairs with `runtime === sessionId`, so whenever it fell outside the selected lineage the scan already refused. The 32 recovery cases pass unchanged. --- .../use-prompt-actions/steering-session.ts | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts index 4c40e817fb..a7fc3638e3 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts @@ -27,25 +27,37 @@ export function captureSteeringSession(deps: SteeringSessionDeps) { deps const sessionId = activeSessionIdRef.current + + if (!sessionId) { + return null + } + const selectedStoredSessionId = selectedStoredSessionIdRef.current const routedStoredSessionId = getRoutedStoredSessionId() const bindings = runtimeIdByStoredSessionIdRef.current - const boundStoredSessionId = sessionId ? findStoredIdForRuntimeId(bindings, sessionId) : undefined + const boundStoredSessionId = findStoredIdForRuntimeId(bindings, sessionId) const sessions = $sessions.get() const matchesSelection = (id: string) => Boolean(selectedStoredSessionId && idsShareLineage(id, selectedStoredSessionId, sessions)) - // Navigation publishes these identities independently. Unlike ordinary Send, - // a mid-turn correction must not resolve another target and interrupt it. - if ( - !sessionId || - (routedStoredSessionId && !matchesSelection(routedStoredSessionId)) || - (selectedStoredSessionId && - selectedStoredSessionId !== sessionId && - (!boundStoredSessionId || !matchesSelection(boundStoredSessionId))) || - [...bindings].some(([stored, runtime]) => runtime === sessionId && !matchesSelection(stored)) - ) { + // Navigation publishes route, selection and runtime independently. Unlike an + // ordinary Send, a mid-turn correction must never resolve another target and + // interrupt it, so any disagreement refuses and the composer queues the text. + const routeLeftSelection = Boolean(routedStoredSessionId && !matchesSelection(routedStoredSessionId)) + + // Selection names a stored chat, yet this runtime proves no binding at all. + const runtimeUnbound = Boolean( + selectedStoredSessionId && selectedStoredSessionId !== sessionId && !boundStoredSessionId + ) + + // Any stored id bound to this runtime outside the selected lineage — which is + // every binding when nothing is selected (a fresh draft next to a live turn). + const runtimeBoundElsewhere = [...bindings].some( + ([stored, runtime]) => runtime === sessionId && !matchesSelection(stored) + ) + + if (routeLeftSelection || runtimeUnbound || runtimeBoundElsewhere) { return null } From 09436570b9fcff7ef67fb167128692bd41d1e7a0 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:03:42 +0530 Subject: [PATCH 03/15] chore(contributors): map obtitus@gmail.com to @obtitus Attribution for the salvage of #116011. --- contributors/emails/obtitus@gmail.com | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 contributors/emails/obtitus@gmail.com diff --git a/contributors/emails/obtitus@gmail.com b/contributors/emails/obtitus@gmail.com new file mode 100644 index 0000000000..052e6d06fc --- /dev/null +++ b/contributors/emails/obtitus@gmail.com @@ -0,0 +1,2 @@ +obtitus +# PR #116011 salvage: browser.backend as a configured provider signal From e6ae3d4a9ea14c5d6b833526a16b6f0f0e92a52d Mon Sep 17 00:00:00 2001 From: obtitus Date: Sat, 19 Sep 2026 23:49:09 +0200 Subject: [PATCH 04/15] fix: recognize browser.backend as a configured provider signal _browser_backend_active and _toolset_needs_configuration_prompt both needed to read the browser backend config key, but each hard-coded the lookup and the YAML 1.1 'off'-parses-as-False normalization separately. Extract _browser_cfg helper so the two share one read path and can't drift. This also adds a fallback check for browser.backend in the selection_key path so the provider picker doesn't re-appear after 'Browser Use' is selected. --- hermes_cli/tools_config_providers.py | 33 ++++++++++++++-- tests/hermes_cli/test_post_setup_gating.py | 44 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/hermes_cli/tools_config_providers.py b/hermes_cli/tools_config_providers.py index e9abd1e900..5830b8946a 100644 --- a/hermes_cli/tools_config_providers.py +++ b/hermes_cli/tools_config_providers.py @@ -224,7 +224,16 @@ def _toolset_needs_configuration_prompt(ts_key: str, config: dict, *, force_fres selection_key = {"tts": "provider", "web": "backend", "browser": "cloud_provider"}.get(ts_key) if selection_key: section = config.get(ts_key, {}) - return not isinstance(section, dict) or selection_key not in section + if not isinstance(section, dict): + return True + if selection_key in section: + return False + # Browser's "Browser Use" provider row writes config[ts_key]["backend"] + # (via the browser_backend marker), not "cloud_provider" — recognize + # an already-set backend so the provider picker doesn't re-appear. + if ts_key == "browser" and _browser_cfg(config, "backend") is not None: + return False + return True if ts_key == "image_gen": # in-tree FAL backend OR any available plugin image gen provider satisfies return not fal_key_is_configured() and not _any_plugin_provider_available("agent.image_gen_registry") if ts_key == "video_gen": # no in-tree fallback — every video backend is a plugin @@ -409,10 +418,26 @@ def _browser_provider_active(provider: dict, config: dict) -> bool: return True +def _browser_cfg(config: dict, key: str): + """Safely read a value from the ``browser`` config section. + + Returns ``None`` when the section or key is absent, or when the section + is not a dict. Normalises YAML 1.1's quirk where an unquoted ``off`` + in ``browser.backend`` parses as boolean ``False`` — this keeps the + single place that needs the workaround. + """ + section = config.get("browser") + if not isinstance(section, dict): + return None + value = section.get(key) + if key == "backend" and value is False: + return "off" + return value + + def _browser_backend_active(provider: dict, config: dict) -> bool: - backend = cfg_get(config, "browser", "backend") - if backend is False: - backend = "off" # YAML 1.1: unquoted `off` parses as boolean False + """Check if a provider entry matches the currently active config.""" + backend = _browser_cfg(config, "backend") if backend == provider["browser_backend"]: return True if backend: diff --git a/tests/hermes_cli/test_post_setup_gating.py b/tests/hermes_cli/test_post_setup_gating.py index 036e872abd..d6c403aa16 100644 --- a/tests/hermes_cli/test_post_setup_gating.py +++ b/tests/hermes_cli/test_post_setup_gating.py @@ -57,3 +57,47 @@ class TestPostSetupGate: monkeypatch.setitem(tools_config._POST_SETUP_INSTALLED, "cua_driver", _boom) assert tools_config._post_setup_already_installed("cua_driver") is True + +class TestBrowserBackendPrompt: + """Regression: `_toolset_needs_configuration_prompt` for the browser toolset + only checked `browser.cloud_provider` (set by `browser_provider` rows), + ignoring `browser.backend` (set by the `browser_backend` "Browser Use" row). + This made the provider picker re-appear every time `hermes tools` was + opened, even when Browser Use was already configured. + """ + + def test_browser_backend_set_skips_provider_picker(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = {"browser": {"backend": "browser-use"}} + assert tools_config._toolset_needs_configuration_prompt("browser", config) is False + + def test_browser_cloud_provider_set_skips_provider_picker(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = {"browser": {"cloud_provider": "local"}} + assert tools_config._toolset_needs_configuration_prompt("browser", config) is False + + def test_browser_unconfigured_still_prompts(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + assert tools_config._toolset_needs_configuration_prompt("browser", {}) is True + + def test_browser_empty_still_prompts(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = {"browser": None} + assert tools_config._toolset_needs_configuration_prompt("browser", config) is True + + def test_browser_backend_off_still_skips_prompt(self, monkeypatch, tmp_path): + """YAML 1.1 parses unquoted `off` as boolean False — the helper must + normalise it, and the gate should still treat it as 'configured'.""" + from hermes_cli import tools_config + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = {"browser": {"backend": False}} # what YAML `off` becomes + assert tools_config._toolset_needs_configuration_prompt("browser", config) is False From e10934b03e115b036b3c38b9c6e6817039a5962d Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:17:20 +0530 Subject: [PATCH 05/15] fix(tools): gate the browser picker on an actual backend choice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_toolset_needs_configuration_prompt` treated "browser.backend is present" as "already configured", but DEFAULT_CONFIG["browser"]["backend"] is "" and load_config() merges defaults — so the key is present on every install and the Browser Automation picker was suppressed everywhere, including a fresh install with no browser config at all. The added tests only fed hand-built dicts, which never go through the merge. Test the value instead of the key, via the falsy check `_browser_backend_active` already uses, and build the helper on the existing `cfg_get` rather than a second nested-traversal implementation. --- hermes_cli/tools_config_providers.py | 31 ++++++---------------- tests/hermes_cli/test_post_setup_gating.py | 30 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/hermes_cli/tools_config_providers.py b/hermes_cli/tools_config_providers.py index 5830b8946a..09ffe32bfa 100644 --- a/hermes_cli/tools_config_providers.py +++ b/hermes_cli/tools_config_providers.py @@ -228,12 +228,9 @@ def _toolset_needs_configuration_prompt(ts_key: str, config: dict, *, force_fres return True if selection_key in section: return False - # Browser's "Browser Use" provider row writes config[ts_key]["backend"] - # (via the browser_backend marker), not "cloud_provider" — recognize - # an already-set backend so the provider picker doesn't re-appear. - if ts_key == "browser" and _browser_cfg(config, "backend") is not None: - return False - return True + # Browser's "Browser Use" row writes browser.backend and leaves cloud_provider unset. Presence is no + # test of a choice here: browser.backend exists on every install after the defaults merge ("" = unset). + return not (ts_key == "browser" and _browser_backend(config)) if ts_key == "image_gen": # in-tree FAL backend OR any available plugin image gen provider satisfies return not fal_key_is_configured() and not _any_plugin_provider_available("agent.image_gen_registry") if ts_key == "video_gen": # no in-tree fallback — every video backend is a plugin @@ -418,26 +415,14 @@ def _browser_provider_active(provider: dict, config: dict) -> bool: return True -def _browser_cfg(config: dict, key: str): - """Safely read a value from the ``browser`` config section. - - Returns ``None`` when the section or key is absent, or when the section - is not a dict. Normalises YAML 1.1's quirk where an unquoted ``off`` - in ``browser.backend`` parses as boolean ``False`` — this keeps the - single place that needs the workaround. - """ - section = config.get("browser") - if not isinstance(section, dict): - return None - value = section.get(key) - if key == "backend" and value is False: - return "off" - return value +def _browser_backend(config: dict) -> str: + """``browser.backend`` as a string; ``""`` when unset or empty (YAML 1.1 parses an unquoted ``off`` as False).""" + backend = cfg_get(config, "browser", "backend") + return "off" if backend is False else (backend or "") def _browser_backend_active(provider: dict, config: dict) -> bool: - """Check if a provider entry matches the currently active config.""" - backend = _browser_cfg(config, "backend") + backend = _browser_backend(config) if backend == provider["browser_backend"]: return True if backend: diff --git a/tests/hermes_cli/test_post_setup_gating.py b/tests/hermes_cli/test_post_setup_gating.py index d6c403aa16..9f1826d080 100644 --- a/tests/hermes_cli/test_post_setup_gating.py +++ b/tests/hermes_cli/test_post_setup_gating.py @@ -101,3 +101,33 @@ class TestBrowserBackendPrompt: monkeypatch.setenv("HERMES_HOME", str(tmp_path)) config = {"browser": {"backend": False}} # what YAML `off` becomes assert tools_config._toolset_needs_configuration_prompt("browser", config) is False + + +class TestBrowserBackendPromptThroughLoader: + """The browser gate must hold through the real config loader. + + `load_config()` merges ``DEFAULT_CONFIG``, where ``browser.backend`` is ``""`` — so the key is + present on every install and a presence test would suppress the picker everywhere. These pin the + behaviour against the merged dict a real ``hermes tools`` run feeds the gate. + """ + + def _home(self, tmp_path, body: str): + (tmp_path / "config.yaml").write_text(body, encoding="utf-8") + return tmp_path + + def test_unset_browser_still_prompts(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + from hermes_cli.config import load_config + + monkeypatch.setenv("HERMES_HOME", str(self._home(tmp_path, "cli: {}\n"))) + config = load_config() + assert config["browser"]["backend"] == "" # defaults merge fills the key + assert tools_config._toolset_needs_configuration_prompt("browser", config) is True + + def test_explicit_backend_skips_prompt(self, monkeypatch, tmp_path): + from hermes_cli import tools_config + from hermes_cli.config import load_config + + monkeypatch.setenv("HERMES_HOME", str(self._home(tmp_path, "browser:\n backend: browser-use\n"))) + config = load_config() + assert tools_config._toolset_needs_configuration_prompt("browser", config) is False From 7d7ee3389f172347e1677c9c3670930e536bf98d Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:12:03 +0530 Subject: [PATCH 06/15] chore: map PuvaanRaaj contributor email Attribution for the #115160 foundation commit carried by #116953. --- .../emails/112681813+PuvaanRaaj@users.noreply.github.com | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 contributors/emails/112681813+PuvaanRaaj@users.noreply.github.com diff --git a/contributors/emails/112681813+PuvaanRaaj@users.noreply.github.com b/contributors/emails/112681813+PuvaanRaaj@users.noreply.github.com new file mode 100644 index 0000000000..b2d92743e7 --- /dev/null +++ b/contributors/emails/112681813+PuvaanRaaj@users.noreply.github.com @@ -0,0 +1,2 @@ +PuvaanRaaj +# PR #115160 / #116953 salvage From 57be2674dd8d35bc18a330fcbfb909f82a93a80f Mon Sep 17 00:00:00 2001 From: Puvaan Raaj <112681813+PuvaanRaaj@users.noreply.github.com> Date: Fri, 18 Sep 2026 23:00:06 +0800 Subject: [PATCH 07/15] fix(desktop): persist preview artifact dismissals per session (cherry picked from commit dc8cd1c34e81c5a9736b5a3cad31f7af41a98686) (cherry picked from commit 70d01e741834910d6f2751da21bfb9280ae8201d) --- .../app/chat/composer/status-stack/index.tsx | 8 ++- .../components/assistant-ui/tool/fallback.tsx | 7 +-- apps/desktop/src/store/preview-status.test.ts | 16 +++++- apps/desktop/src/store/preview-status.ts | 51 ++++++++++++++++++- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index d4eba9eafa..62a351fd6d 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/index.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/index.tsx @@ -5,6 +5,7 @@ import { useNavigate } from 'react-router' import { blurComposerInput } from '@/app/chat/composer/focus' import { useComposerSurfaceId } from '@/app/chat/composer/scope' import { AGENTS_ROUTE } from '@/app/routes' +import { useSessionView } from '@/app/chat/session-view' import type { SubmitTextOptions } from '@/app/session/hooks/use-prompt-actions/utils' import { BillingBanner } from '@/components/billing-banner' import { composerDockCard } from '@/components/chat/composer-dock' @@ -97,6 +98,7 @@ interface ComposerStatusStackProps { export function ComposerStatusStack({ onSubmit, queue, sessionId }: ComposerStatusStackProps) { const { t } = useI18n() const navigate = useNavigate() + const storedSessionId = useStore(useSessionView().$storedId) useSubagentSnapshot(sessionId) // Subscribe to THIS session's slice only. Both maps churn on other // sessions' activity (subagent ticks, background polls, preview updates in @@ -185,7 +187,11 @@ export function ComposerStatusStack({ onSubmit, queue, sessionId }: ComposerStat const previewRows = visiblePreviews.length > 0 && sessionId ? visiblePreviews.map(item => ( - dismissPreviewArtifact(sessionId, id)} /> + dismissPreviewArtifact(sessionId, id, storedSessionId ?? sessionId)} + /> )) : [] diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 7aacf22101..898b84e5bd 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -418,7 +418,7 @@ function ToolEntry({ part }: ToolEntryProps) { const previewTarget = view.previewTarget // The session whose transcript this row is IN, which is not necessarily the // primary one: a tool row inside a session tile must feed that tile's composer. - const { $cwd: $sessionCwd, $runtimeId: $sessionRuntimeId } = useSessionView() + const { $cwd: $sessionCwd, $runtimeId: $sessionRuntimeId, $storedId: $sessionStoredId } = useSessionView() useEffect(() => { if (isPending || !previewTarget || !isPreviewableTarget(previewTarget)) { @@ -429,11 +429,12 @@ function ToolEntry({ part }: ToolEntryProps) { // target appears, and subscribing re-rendered every tool row on any session // or cwd change. const sessionId = $sessionRuntimeId.get() + const dismissalSessionId = $sessionStoredId?.get() ?? sessionId if (sessionId) { - recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '') + recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '', dismissalSessionId ?? '') } - }, [$sessionCwd, $sessionRuntimeId, isPending, previewTarget]) + }, [$sessionCwd, $sessionRuntimeId, $sessionStoredId, isPending, previewTarget]) const detailSections = useMemo(() => { if (!view.detail) { diff --git a/apps/desktop/src/store/preview-status.test.ts b/apps/desktop/src/store/preview-status.test.ts index e9ffbf322a..522eaedbd8 100644 --- a/apps/desktop/src/store/preview-status.test.ts +++ b/apps/desktop/src/store/preview-status.test.ts @@ -7,7 +7,10 @@ import { recordPreviewArtifact } from './preview-status' -beforeEach(() => $previewStatusBySession.set({})) +beforeEach(() => { + window.localStorage.clear() + $previewStatusBySession.set({}) +}) describe('recordPreviewArtifact', () => { it('appends new targets newest-last and is idempotent', () => { @@ -38,4 +41,15 @@ describe('recordPreviewArtifact', () => { clearPreviewArtifacts('s1') expect($previewStatusBySession.get().s1).toBeUndefined() }) + + it('suppresses a dismissed historical target when the row mounts again', () => { + recordPreviewArtifact('runtime-a', '/a/index.html', '/work', 'stored-a') + dismissPreviewArtifact('runtime-a', '/a/index.html', 'stored-a') + $previewStatusBySession.set({}) + + recordPreviewArtifact('runtime-b', '/a/index.html', '/work', 'stored-a') + + expect($previewStatusBySession.get()['runtime-b']).toBeUndefined() + expect(window.localStorage.getItem('hermes.desktop.previewDismissals.v1')).toContain('stored-a') + }) }) diff --git a/apps/desktop/src/store/preview-status.ts b/apps/desktop/src/store/preview-status.ts index 618f06f7bd..a457b6ba6c 100644 --- a/apps/desktop/src/store/preview-status.ts +++ b/apps/desktop/src/store/preview-status.ts @@ -1,6 +1,8 @@ import { atom } from 'nanostores' +import { activeConnectionScopeSuffix } from '@/lib/connection-scoped' import { previewName } from '@/lib/preview-targets' +import { readJson, writeJson } from '@/lib/storage' /** * Session-scoped feed of previewable artifacts (HTML files, localhost dev URLs) @@ -21,9 +23,46 @@ export interface PreviewArtifact { } const MAX_PER_SESSION = 4 +const DISMISSED_PREVIEWS_KEY = 'hermes.desktop.previewDismissals.v1' export const $previewStatusBySession = atom>({}) +type DismissedPreviewIds = Record + +function dismissedPreviewsKey(): string { + return `${DISMISSED_PREVIEWS_KEY}${activeConnectionScopeSuffix()}` +} + +function readDismissedPreviewIds(): DismissedPreviewIds { + const value = readJson(dismissedPreviewsKey()) + + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return {} + } + + return Object.fromEntries( + Object.entries(value).flatMap(([sid, ids]) => { + if (!Array.isArray(ids)) { + return [] + } + + const strings = ids.filter((id): id is string => typeof id === 'string' && id.length > 0) + return strings.length > 0 ? [[sid, strings]] : [] + }) + ) +} + +function rememberDismissedPreview(sid: string, id: string): void { + const dismissed = readDismissedPreviewIds() + const ids = dismissed[sid] ?? [] + + if (ids.includes(id)) { + return + } + + writeJson(dismissedPreviewsKey(), { ...dismissed, [sid]: [...ids, id] }) +} + const writePreviews = (sid: string, items: PreviewArtifact[]) => { const current = $previewStatusBySession.get() @@ -47,7 +86,7 @@ const writePreviews = (sid: string, items: PreviewArtifact[]) => { * in the list keeps its slot (the tool row re-registers on every render, so this * must not churn the atom or reorder rows). */ -export function recordPreviewArtifact(sid: string, target: string, cwd: string) { +export function recordPreviewArtifact(sid: string, target: string, cwd: string, dismissalSid = sid) { const raw = target.trim() if (!sid || !raw) { @@ -60,10 +99,14 @@ export function recordPreviewArtifact(sid: string, target: string, cwd: string) return } + if (readDismissedPreviewIds()[dismissalSid]?.includes(raw)) { + return + } + writePreviews(sid, [...list, { cwd, id: raw, label: previewName(raw), target: raw }].slice(-MAX_PER_SESSION)) } -export function dismissPreviewArtifact(sid: string, id: string) { +export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = sid) { const list = $previewStatusBySession.get()[sid] if (list) { @@ -72,6 +115,10 @@ export function dismissPreviewArtifact(sid: string, id: string) { list.filter(item => item.id !== id) ) } + + if (dismissalSid && id) { + rememberDismissedPreview(dismissalSid, id) + } } export function clearPreviewArtifacts(sid: string) { From 30f0b220214c2e7aa8bd9642f7e008e14c7d913d Mon Sep 17 00:00:00 2001 From: BearHuddleston Date: Sun, 20 Sep 2026 01:29:07 -0500 Subject: [PATCH 08/15] fix(desktop): honor artifact dismissal across navigation and replay (cherry picked from commit 4f9b2e783cde917080307566e7200db13434a1b8) --- .../app/chat/composer/status-stack/index.tsx | 2 +- .../gateway-event/tools-preview.test.ts | 178 ++++++++++++ .../use-message-stream/gateway-event/tools.ts | 33 +++ .../assistant-ui/tool/fallback-model/index.ts | 5 + .../tool/fallback-model/targets.ts | 5 + .../tool/fallback-preview-scope.test.tsx | 39 ++- .../components/assistant-ui/tool/fallback.tsx | 32 ++- apps/desktop/src/lib/preview-targets.ts | 46 ++++ .../store/preview-status-edge-cases.test.ts | 49 ++++ .../store/preview-status-lifecycle.test.ts | 63 +++++ apps/desktop/src/store/preview-status.test.ts | 40 +++ apps/desktop/src/store/preview-status.ts | 255 ++++++++++++++++-- apps/desktop/src/store/session-states.ts | 3 + apps/shared/src/gateway-events.ts | 2 + apps/shared/src/json-rpc-gateway.ts | 4 +- website/docs/user-guide/desktop.md | 2 + 16 files changed, 725 insertions(+), 33 deletions(-) create mode 100644 apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools-preview.test.ts create mode 100644 apps/desktop/src/store/preview-status-edge-cases.test.ts create mode 100644 apps/desktop/src/store/preview-status-lifecycle.test.ts diff --git a/apps/desktop/src/app/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index 62a351fd6d..5ead6286be 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/index.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/index.tsx @@ -4,8 +4,8 @@ import { useNavigate } from 'react-router' import { blurComposerInput } from '@/app/chat/composer/focus' import { useComposerSurfaceId } from '@/app/chat/composer/scope' -import { AGENTS_ROUTE } from '@/app/routes' import { useSessionView } from '@/app/chat/session-view' +import { AGENTS_ROUTE } from '@/app/routes' import type { SubmitTextOptions } from '@/app/session/hooks/use-prompt-actions/utils' import { BillingBanner } from '@/components/billing-banner' import { composerDockCard } from '@/components/chat/composer-dock' diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools-preview.test.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools-preview.test.ts new file mode 100644 index 0000000000..6f8bcc764d --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools-preview.test.ts @@ -0,0 +1,178 @@ +import { type GatewayEvent, JsonRpcGatewayClient } from '@hermes/shared' +import { afterEach, expect, it, vi } from 'vitest' + +import { type GatewayEventPayload, upsertToolPart } from '@/lib/chat-messages' +import { createClientSessionState } from '@/lib/chat-runtime' +import { $previewStatusBySession, dismissPreviewArtifact, recordPreviewArtifact } from '@/store/preview-status' +import { + $sessionStates, + clearAllSessionStates, + publishSessionState, + recordSessionEventScope +} from '@/store/session-states' + +import { handleToolEvent } from './tools' +import type { GatewayEventContext } from './types' + +const sid = 'preview-production' +const args = { path: '/work/report.html' } + +function pending() { + const state = createClientSessionState('production-stored') + state.busy = true + state.messages = [ + { + id: 'live-tool-message', + role: 'assistant', + parts: upsertToolPart( + [], + { + name: 'write_file', + tool_id: 'reused-tool-id', + args + }, + 'running', + 1 + ) + } + ] + publishSessionState(sid, state) +} + +function event(seq: number, result: unknown = { verified: true }): GatewayEvent<'tool.complete'> { + return { + type: 'tool.complete', + session_id: sid, + seq, + payload: { name: 'write_file', tool_id: 'reused-tool-id', args, result } + } +} + +function deliver(event: GatewayEvent<'tool.complete'>) { + return handleToolEvent({ + event, + payload: event.payload, + sessionId: sid, + occurredAt: 2, + isActiveEvent: false, + deps: { + flushQueuedDeltas: vi.fn(), + updateSessionState: vi.fn(), + sessionInterrupted: () => false, + upsertToolCall: () => { + const state = $sessionStates.get()[sid] + + if (state) { + publishSessionState(sid, { + ...state, + messages: state.messages.map(message => ({ + ...message, + parts: upsertToolPart(message.parts, event.payload as GatewayEventPayload, 'complete', 2) + })) + }) + } + } + } + } as unknown as GatewayEventContext) +} + +afterEach(() => { + $previewStatusBySession.set({}) + clearAllSessionStates() + window.localStorage.clear() +}) + +it('reoffers a real later production, never a historical mount or duplicate completion', () => { + recordSessionEventScope({ session_id: sid, connectionId: 'local', profile: 'default' }) + pending() + deliver(event(10)) + expect($previewStatusBySession.get()[sid]).toHaveLength(1) + dismissPreviewArtifact(sid, '/work/report.html') + recordPreviewArtifact(sid, 'file:///work/report.html', '/work') + deliver(event(10)) + pending() + deliver(event(11, { error: 'Permission denied' })) + expect($previewStatusBySession.get()[sid]).toBeUndefined() + pending() + deliver(event(12)) + expect($previewStatusBySession.get()[sid]).toHaveLength(1) + dismissPreviewArtifact(sid, '/work/report.html') + deliver(event(12)) + expect($previewStatusBySession.get()[sid]).toBeUndefined() +}) + +class Socket extends EventTarget { + readyState = 0 + sent: string[] = [] + send(data: string) { + this.sent.push(data) + } + close() { + this.readyState = 3 + this.dispatchEvent(new CloseEvent('close')) + } + open() { + this.readyState = 1 + this.dispatchEvent(new Event('open')) + } + frame(data: unknown) { + this.dispatchEvent(new MessageEvent('message', { data: JSON.stringify(data) })) + } +} + +it('keeps dismissal when a missed completion arrives through the real reconnect replay path', async () => { + const sockets: Socket[] = [] + + const client = new JsonRpcGatewayClient({ + heartbeatIntervalMs: 0, + heartbeatDeadlineMs: 0, + socketFactory: () => { + const socket = new Socket() + sockets.push(socket) + + return socket as unknown as WebSocket + } + }) + + client.on('tool.complete', deliver) + + try { + let connecting = client.connect('ws://fixture.invalid') + sockets[0].open() + await connecting + sockets[0].frame({ + jsonrpc: '2.0', + method: 'event', + params: { type: 'message.delta', session_id: sid, seq: 1, payload: { text: '' } } + }) + client.invalidate('fixture disconnect') + connecting = client.connect('ws://fixture.invalid') + sockets[1].open() + await connecting + await vi.waitFor(() => + expect(sockets[1].sent.map(text => JSON.parse(text).method)).toContain('session.events.since') + ) + const request = sockets[1].sent.map(text => JSON.parse(text)).find(value => value.method === 'session.events.since') + recordSessionEventScope({ session_id: sid, connectionId: 'local', profile: 'default' }) + pending() + recordPreviewArtifact(sid, '/work/report.html', '/work', 'production-stored') + dismissPreviewArtifact(sid, '/work/report.html', 'production-stored') + const seen = vi.fn() + client.on('tool.complete', seen) + sockets[1].frame({ + jsonrpc: '2.0', + id: request.id, + result: { + events: [event(2)], + latest_seq: 2, + truncated: false, + count: 1 + } + }) + await vi.waitFor(() => expect(seen).toHaveBeenCalledOnce()) + expect(seen.mock.calls[0][0].replayed).toBe(true) + expect($previewStatusBySession.get()[sid]).toBeUndefined() + } finally { + client.close() + } +}) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts index 4b919e863a..8d533deedc 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts @@ -1,7 +1,11 @@ +import { buildToolView, isPreviewableTarget } from '@/components/assistant-ui/tool/fallback-model' import { reportFirstBuildToolComplete } from '@/components/onboarding-chat/first-build' +import { toolCallOwnerMessageId } from '@/lib/chat-messages' import { invalidateSlashCompletions } from '@/lib/slash-completion-cache' import { refreshBackgroundProcesses } from '@/store/composer-status' import { flashPetActivity, setPetActivity } from '@/store/pet' +import { recordPreviewArtifact } from '@/store/preview-status' +import { $sessionStates, storedSessionIdForRuntimeId } from '@/store/session-states' import { pruneDelegateFallbackSubagents, upsertSubagent } from '@/store/subagents' import { reportMcpToolResult } from '@/store/suggestion-providers/repair' import { invalidateSkillSuggestionIndex } from '@/store/suggestion-providers/skill' @@ -68,7 +72,36 @@ export function handleToolEvent(ctx: GatewayEventContext): boolean { if (event.type === 'tool.complete') { if (sessionId) { flushQueuedDeltas(sessionId) + + const pendingProduction = + !event.replayed && Boolean(toolCallOwnerMessageId($sessionStates.get()[sessionId]?.messages ?? [], payload)) + upsertToolCall(sessionId, toTodoPayload(payload) ?? payload, 'complete', event.type, occurredAt) + + if (!sessionInterrupted(sessionId) && payload?.name && payload.result !== undefined && event.seq !== undefined) { + const view = buildToolView( + { + type: 'tool-call', + toolName: payload.name, + args: payload.args ?? {}, + result: payload.result, + toolResultMetadata: payload, + completedAt: occurredAt + }, + '' + ) + + if (view.status === 'success' && view.previewTarget && isPreviewableTarget(view.previewTarget)) { + recordPreviewArtifact( + sessionId, + view.previewTarget, + $sessionStates.get()[sessionId]?.cwd ?? '', + storedSessionIdForRuntimeId(sessionId) ?? sessionId, + pendingProduction + ) + } + } + // Onboarding's first build paces its check-ins off real work done // (no-op in every other session). reportFirstBuildToolComplete(sessionId) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index a0aaed80ca..468ce141bd 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts @@ -751,6 +751,11 @@ function durationLabel(resultRecord: Record): string | undefine } function toolPreviewTarget(toolName: string, args: Record, result: Record): string { + // Reading an existing file is not producing a deliverable. + if (toolName === 'read_file' || toolName === 'search_files' || toolName === 'list_files') { + return '' + } + const direct = firstStringField(result, ['preview', 'url', 'target']) || firstStringField(args, ['preview', 'url', 'target', 'path', 'file', 'filepath']) || diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts index c2168f0502..01605f80b4 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/targets.ts @@ -9,6 +9,11 @@ export function looksLikePath(value: string): boolean { } export function isPreviewableTarget(target: string): boolean { + // Renderer metadata is not a deliverable; app.asar.unpacked is a real directory. + if (/^file:\/\//i.test(target) && target.replace(/\\/g, '/').split('/').includes('app.asar')) { + return false + } + return Boolean( target && (/^file:\/\//i.test(target) || diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx index 19c0868be3..c853f565bf 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx @@ -4,8 +4,9 @@ import type { ComponentProps, ReactNode } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import { type SessionView, SessionViewProvider } from '@/app/chat/session-view' +import type { ChatMessage } from '@/lib/chat-messages' import { $previewStatusBySession } from '@/store/preview-status' -import { $activeSessionId, $currentCwd } from '@/store/session' +import { $activeSessionId, $currentCwd, $messages } from '@/store/session' vi.mock('@assistant-ui/react', async importOriginal => ({ ...(await importOriginal>()), @@ -17,24 +18,26 @@ const { ToolFallback } = await import('./fallback') const PRIMARY_ID = 'primary-session' const TILE_ID = 'tile-session' +const messages: ChatMessage[] = [{ id: 'msg-1', role: 'assistant', parts: [] }] /** Minimal tile view: only the fields the tool row reads. */ function tileView(): SessionView { return { ...({} as SessionView), $cwd: atom('/tile/work'), - $messages: atom([]), + $messages: atom(messages), $runtimeId: atom(TILE_ID), kind: 'tile' } } -function renderToolRow(wrap: (node: ReactNode) => ReactNode) { +function renderToolRow(wrap: (node: ReactNode) => ReactNode, overrides: Record = {}) { const props = { args: { path: '/tile/work/report.html' }, result: { path: '/tile/work/report.html' }, toolCallId: 'call-1', - toolName: 'write_file' + toolName: 'write_file', + ...overrides } as unknown as ComponentProps render(<>{wrap()}) @@ -45,6 +48,7 @@ afterEach(() => { $previewStatusBySession.set({}) $activeSessionId.set(null) $currentCwd.set('') + $messages.set([]) }) describe('tool row preview recording', () => { @@ -68,9 +72,36 @@ describe('tool row preview recording', () => { it('still records into the primary session for the main chat', () => { $activeSessionId.set(PRIMARY_ID) $currentCwd.set('/primary/work') + $messages.set(messages) renderToolRow(node => node) expect(Object.keys($previewStatusBySession.get())).toEqual([PRIMARY_ID]) }) + + it('does not promote reads, failed writes or packaged renderer URLs into artifacts', () => { + $activeSessionId.set(PRIMARY_ID) + $messages.set(messages) + + for (const overrides of [ + { toolName: 'read_file' }, + { isError: true, result: { error: 'Permission denied' } }, + { args: { path: '/work/missing.html' }, result: undefined }, + { result: { preview: 'file:///opt/Hermes/resources/app.asar/dist/index.html' } } + ]) { + renderToolRow(node => node, overrides) + expect($previewStatusBySession.get()[PRIMARY_ID]).toBeUndefined() + cleanup() + } + + renderToolRow(node => node) + expect($previewStatusBySession.get()[PRIMARY_ID]).toHaveLength(1) + }) + + it('does not register a previous conversation row under the newly selected chat', () => { + $activeSessionId.set('next-conversation') + $messages.set([{ id: 'next-message', role: 'assistant', parts: [] }]) + renderToolRow(node => node) + expect($previewStatusBySession.get()['next-conversation']).toBeUndefined() + }) }) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 898b84e5bd..54d98fa79d 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -418,10 +418,22 @@ function ToolEntry({ part }: ToolEntryProps) { const previewTarget = view.previewTarget // The session whose transcript this row is IN, which is not necessarily the // primary one: a tool row inside a session tile must feed that tile's composer. - const { $cwd: $sessionCwd, $runtimeId: $sessionRuntimeId, $storedId: $sessionStoredId } = useSessionView() + + const { + $cwd: $sessionCwd, + $runtimeId: $sessionRuntimeId, + $storedId: $sessionStoredId, + $messages: $sessionMessages + } = useSessionView() useEffect(() => { - if (isPending || !previewTarget || !isPreviewableTarget(previewTarget)) { + if ( + isPending || + result === undefined || + view.status !== 'success' || + !previewTarget || + !isPreviewableTarget(previewTarget) + ) { return } @@ -431,10 +443,22 @@ function ToolEntry({ part }: ToolEntryProps) { const sessionId = $sessionRuntimeId.get() const dismissalSessionId = $sessionStoredId?.get() ?? sessionId - if (sessionId) { + // A route switch can paint the previous assistant row while these atoms + // already describe the next chat. Only that chat's own messages may feed it. + if (sessionId && $sessionMessages.get().some(message => message.id === messageId)) { recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '', dismissalSessionId ?? '') } - }, [$sessionCwd, $sessionRuntimeId, $sessionStoredId, isPending, previewTarget]) + }, [ + $sessionCwd, + $sessionRuntimeId, + $sessionStoredId, + $sessionMessages, + isPending, + messageId, + previewTarget, + result, + view.status + ]) const detailSections = useMemo(() => { if (!view.detail) { diff --git a/apps/desktop/src/lib/preview-targets.ts b/apps/desktop/src/lib/preview-targets.ts index 3b6dfe78b3..8e16b3cc24 100644 --- a/apps/desktop/src/lib/preview-targets.ts +++ b/apps/desktop/src/lib/preview-targets.ts @@ -52,6 +52,52 @@ export function previewName(target: string): string { } } +/** File identity only: do not resolve symlinks, guess home, or fold path case. */ +export function previewArtifactKey(target: string, cwd: string): string { + let path = target.trim() + + if (/^https?:\/\//i.test(path)) { + return path + } + + if (/^file:\/\//i.test(path)) { + try { + const url = new URL(path) + + // Encoded separators are not legal file-URL path segments. + if (/%2f|%5c/i.test(url.pathname)) { + return path + } + + path = decodeURIComponent(url.pathname) + + if (url.hostname) { + path = `//${url.hostname}${path}` + } else if (/^\/[a-z]:\//i.test(path)) { + path = path.slice(1) + } + } catch { + return path + } + } + + const windows = /^[a-z]:[\\/]/i.test(path) || path.startsWith('\\\\') + + if (windows) { + path = path.replace(/\\/g, '/') + } + + if (!/^(?:\/|~\/|[a-z]:\/)/i.test(path) && cwd) { + path = `${cwd.replace(/\/$/, '')}/${path.replace(/^\.\//, '')}` + } + + if (/^[a-z]:[\\/]/i.test(path) || path.startsWith('\\\\')) { + path = path.replace(/\\/g, '/') + } + + return path.replace(/\/\.\//g, '/') +} + export function previewDisplayLabel(target: string): string { const escaped = previewName(target).replace(/[[\]\\]/g, '\\$&') diff --git a/apps/desktop/src/store/preview-status-edge-cases.test.ts b/apps/desktop/src/store/preview-status-edge-cases.test.ts new file mode 100644 index 0000000000..835c481875 --- /dev/null +++ b/apps/desktop/src/store/preview-status-edge-cases.test.ts @@ -0,0 +1,49 @@ +import { afterEach, expect, it } from 'vitest' + +import { createClientSessionState } from '@/lib/chat-runtime' +import { makeSessionInfo } from '@/test/session-info' + +import { $previewStatusBySession, dismissPreviewArtifact, recordPreviewArtifact } from './preview-status' +import { setSessions } from './session' +import { + clearAllSessionStates, + migrateTilesForProfile, + publishSessionState, + recordSessionEventScope +} from './session-states' + +afterEach(() => { + $previewStatusBySession.set({}) + setSessions([]) + clearAllSessionStates() + window.localStorage.clear() +}) + +it('retains a close when the same runtime owner is refined from profile-only to an exact route', () => { + setSessions([makeSessionInfo({ id: 'refined-stored', profile: 'probe' })]) + publishSessionState('refined-runtime', createClientSessionState('refined-stored')) + recordPreviewArtifact('refined-runtime', '/work/refined.html', '/work', 'refined-stored') + recordSessionEventScope({ session_id: 'refined-runtime', connectionId: 'local', profile: 'probe' }) + dismissPreviewArtifact('refined-runtime', '/work/refined.html', 'refined-stored') + recordPreviewArtifact('refined-runtime', '/work/refined.html', '/work', 'refined-stored') + expect($previewStatusBySession.get()['refined-runtime']).toBeUndefined() + + setSessions([makeSessionInfo({ id: 'refined-append-stored', profile: 'probe-append' })]) + publishSessionState('refined-append', createClientSessionState('refined-append-stored')) + recordPreviewArtifact('refined-append', '/work/first.html', '/work', 'refined-append-stored') + recordSessionEventScope({ session_id: 'refined-append', connectionId: 'local', profile: 'probe-append' }) + recordPreviewArtifact('refined-append', '/work/second.html', '/work', 'refined-append-stored') + dismissPreviewArtifact('refined-append', '/work/first.html', 'refined-append-stored') + recordPreviewArtifact('refined-append', '/work/first.html', '/work', 'refined-append-stored') + expect($previewStatusBySession.get()['refined-append'].map(item => item.id)).toEqual(['/work/second.html']) +}) + +it('tolerates malformed persisted scope keys and keeps Windows aliases identical', () => { + window.localStorage.setItem('hermes.desktop.previewDismissals.v1', '{"__proto__":["x"],"constructor":["y"]}') + expect(() => migrateTilesForProfile('old-profile', 'new-profile')).not.toThrow() + recordPreviewArtifact('windows-alias', './report.html', 'C:\\work') + const item = $previewStatusBySession.get()['windows-alias'][0] + dismissPreviewArtifact('windows-alias', item.id) + recordPreviewArtifact('windows-alias', 'file:///C:/work/report.html', 'C:\\work') + expect($previewStatusBySession.get()['windows-alias']).toBeUndefined() +}) diff --git a/apps/desktop/src/store/preview-status-lifecycle.test.ts b/apps/desktop/src/store/preview-status-lifecycle.test.ts new file mode 100644 index 0000000000..a482a72033 --- /dev/null +++ b/apps/desktop/src/store/preview-status-lifecycle.test.ts @@ -0,0 +1,63 @@ +import { afterEach, expect, it, vi } from 'vitest' + +import { + $previewStatusBySession, + clearPreviewArtifacts, + dismissPreviewArtifact, + recordPreviewArtifact +} from './preview-status' +import { dropTilesForProfile, migrateTilesForProfile, recordSessionEventScope } from './session-states' + +afterEach(() => { + vi.restoreAllMocks() + $previewStatusBySession.set({}) + window.localStorage.clear() +}) + +it('moves local dismissals on rename and removes only the deleted owner', () => { + for (const [runtime, connectionId] of [ + ['rename-local', 'local'], + ['rename-remote', 'remote'] + ]) { + recordSessionEventScope({ session_id: runtime, connectionId, profile: 'before-rename' }) + recordPreviewArtifact(runtime, '/work/rename.html', '/work', 'rename-stored') + dismissPreviewArtifact(runtime, '/work/rename.html', 'rename-stored') + } + + migrateTilesForProfile('before-rename', 'after-rename') + recordSessionEventScope({ session_id: 'rename-new', connectionId: 'local', profile: 'after-rename' }) + recordPreviewArtifact('rename-new', '/work/rename.html', '/work', 'rename-stored') + recordPreviewArtifact('rename-remote', '/work/rename.html', '/work', 'rename-stored') + expect($previewStatusBySession.get()['rename-new']).toBeUndefined() + expect($previewStatusBySession.get()['rename-remote']).toBeUndefined() + dropTilesForProfile('after-rename') + recordPreviewArtifact('rename-new', '/work/rename.html', '/work', 'rename-stored') + recordPreviewArtifact('rename-remote', '/work/rename.html', '/work', 'rename-stored') + expect($previewStatusBySession.get()['rename-new']).toHaveLength(1) + expect($previewStatusBySession.get()['rename-remote']).toBeUndefined() +}) + +it('honors memory-only closes on storage failure and durable closes after module reload', async () => { + recordSessionEventScope({ session_id: 'quota', connectionId: 'local', profile: 'quota' }) + recordPreviewArtifact('quota', '/work/quota.html', '/work', 'quota-stored') + + const write = vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new Error('quota') + }) + + dismissPreviewArtifact('quota', '/work/quota.html', 'quota-stored') + clearPreviewArtifacts('quota') + recordPreviewArtifact('quota', '/work/quota.html', '/work', 'quota-stored') + expect($previewStatusBySession.get().quota).toBeUndefined() + write.mockRestore() + + recordSessionEventScope({ session_id: 'before-reload', connectionId: 'local', profile: 'persist' }) + recordPreviewArtifact('before-reload', '/work/saved.html', '/work', 'persist-stored') + dismissPreviewArtifact('before-reload', '/work/saved.html', 'persist-stored') + vi.resetModules() + const fresh = await import('./preview-status') + const { recordSessionEventScope: scope } = await import('./session-states') + scope({ session_id: 'after-reload', connectionId: 'local', profile: 'persist' }) + fresh.recordPreviewArtifact('after-reload', '/work/saved.html', '/work', 'persist-stored') + expect(fresh.$previewStatusBySession.get()['after-reload']).toBeUndefined() +}) diff --git a/apps/desktop/src/store/preview-status.test.ts b/apps/desktop/src/store/preview-status.test.ts index 522eaedbd8..41c7622899 100644 --- a/apps/desktop/src/store/preview-status.test.ts +++ b/apps/desktop/src/store/preview-status.test.ts @@ -1,11 +1,14 @@ import { beforeEach, describe, expect, it } from 'vitest' +import { rescopeConnectionScopedStores } from '@/lib/connection-scoped' + import { $previewStatusBySession, clearPreviewArtifacts, dismissPreviewArtifact, recordPreviewArtifact } from './preview-status' +import { recordSessionEventScope } from './session-states' beforeEach(() => { window.localStorage.clear() @@ -43,6 +46,10 @@ describe('recordPreviewArtifact', () => { }) it('suppresses a dismissed historical target when the row mounts again', () => { + for (const session_id of ['runtime-a', 'runtime-b']) { + recordSessionEventScope({ session_id, connectionId: 'owner', profile: 'default' }) + } + recordPreviewArtifact('runtime-a', '/a/index.html', '/work', 'stored-a') dismissPreviewArtifact('runtime-a', '/a/index.html', 'stored-a') $previewStatusBySession.set({}) @@ -52,4 +59,37 @@ describe('recordPreviewArtifact', () => { expect($previewStatusBySession.get()['runtime-b']).toBeUndefined() expect(window.localStorage.getItem('hermes.desktop.previewDismissals.v1')).toContain('stored-a') }) + + it('keeps dismissal with the session owner across foreground changes and runtime rebinds', () => { + const record = (runtime: string, connectionId: string, profile: string) => { + recordSessionEventScope({ session_id: runtime, connectionId, profile }) + recordPreviewArtifact(runtime, '/work/report.html', '/work', 'same-stored-id') + } + + record('owner-a', 'local', 'alpha') + dismissPreviewArtifact('owner-a', '/work/report.html', 'same-stored-id') + $previewStatusBySession.set({}) + rescopeConnectionScopedStores({ mode: 'remote', baseUrl: 'https://other.invalid', profile: 'other' }) + record('owner-a-rebound', 'local', 'alpha') + record('other-profile', 'local', 'beta') + record('other-connection', 'remote', 'alpha') + expect($previewStatusBySession.get()['owner-a-rebound']).toBeUndefined() + expect($previewStatusBySession.get()['other-profile']).toHaveLength(1) + expect($previewStatusBySession.get()['other-connection']).toHaveLength(1) + rescopeConnectionScopedStores({ mode: 'local' }) + }) + + it('deduplicates equivalent file URLs without hiding distinct same-named files', () => { + recordPreviewArtifact('files', '/work/one/index.html', '/work') + recordPreviewArtifact('files', 'file:///work/one/index.html', '/work') + recordPreviewArtifact('files', './one/index.html', '/work') + recordPreviewArtifact('files', '/work/two/index.html', '/work') + + const items = $previewStatusBySession.get().files + expect(items).toHaveLength(2) + expect(new Set(items.map(item => item.label)).size).toBe(2) + dismissPreviewArtifact('files', items[0].id) + recordPreviewArtifact('files', 'file:///work/one/index.html', '/work') + expect($previewStatusBySession.get().files).toHaveLength(1) + }) }) diff --git a/apps/desktop/src/store/preview-status.ts b/apps/desktop/src/store/preview-status.ts index a457b6ba6c..f9d62a97c0 100644 --- a/apps/desktop/src/store/preview-status.ts +++ b/apps/desktop/src/store/preview-status.ts @@ -1,9 +1,12 @@ import { atom } from 'nanostores' -import { activeConnectionScopeSuffix } from '@/lib/connection-scoped' -import { previewName } from '@/lib/preview-targets' +import { previewArtifactKey, previewName } from '@/lib/preview-targets' import { readJson, writeJson } from '@/lib/storage' +import { ownerLookupSessionRows, resolveComposerSessionKey } from './session' +import type { SessionOwnerRoute } from './session-request-router' +import { knownOwnerForSession, runtimeSessionOwner, storedSessionIdForRuntimeId } from './session-states' + /** * Session-scoped feed of previewable artifacts (HTML files, localhost dev URLs) * a tool produced. Surfaced as compact links in the composer status stack — @@ -16,51 +19,211 @@ import { readJson, writeJson } from '@/lib/storage' export interface PreviewArtifact { /** cwd captured at detection so a relative path still resolves on click. */ cwd: string - /** Dedupe key + display id (the raw target). */ + /** Canonical target identity, independent of its display label. */ id: string label: string target: string + /** Captured owner; foreground switches must not retarget a later dismiss. */ + dismissalScope?: string } const MAX_PER_SESSION = 4 const DISMISSED_PREVIEWS_KEY = 'hermes.desktop.previewDismissals.v1' +const MAX_DISMISSED_SESSIONS = 128 +const MAX_DISMISSED_TARGETS = 64 export const $previewStatusBySession = atom>({}) -type DismissedPreviewIds = Record +interface DismissedPreviewIds { + [session: string]: string[] +} +const volatileDismissals = new Map() +const scopeByRuntime = new Map() -function dismissedPreviewsKey(): string { - return `${DISMISSED_PREVIEWS_KEY}${activeConnectionScopeSuffix()}` +function dismissalKey(runtimeId: string, storedId: string): string { + // The route's stored selection can advance before the old runtime unmounts. + storedId = storedSessionIdForRuntimeId(runtimeId) ?? storedId + // Historical rows in background tiles do not belong to the active connection. + const owner = knownOwnerForSession(runtimeId) ?? runtimeSessionOwner(runtimeId) ?? knownOwnerForSession(storedId) + const profile = typeof owner === 'string' ? owner : owner?.targetProfile || owner?.profile + + const rows = ownerLookupSessionRows().filter( + row => + row.profile === profile && + (row.connection_id || 'local') === (typeof owner === 'object' && owner ? owner.connectionId : 'local') + ) + + const stableId = resolveComposerSessionKey(storedId, rows) ?? storedId + + // Bare profiles name the legacy profile pool, not the foreground connection. + const scope = + owner && typeof owner === 'object' + ? JSON.stringify([owner.connectionId, owner.profile, owner.targetProfile || owner.profile, stableId]) + : typeof owner === 'string' + ? JSON.stringify([null, owner, owner, stableId]) + : JSON.stringify(['runtime', runtimeId]) + + const previous = scopeByRuntime.get(runtimeId) + const before = previous ? storedOwner(previous) : null + const after = storedOwner(scope) + + if ( + previous && + previous !== scope && + after && + (previous === JSON.stringify(['runtime', runtimeId]) || + (before && before[0] === null && before[1] === after[1] && before[2] === after[2] && before[3] === after[3])) + ) { + // Only refinement observed on THIS runtime joins keys. Another profile or + // connection with a coincidentally equal stored id must not inherit it. + rewriteDismissalScopes(key => (key === previous ? scope : key)) + } + + scopeByRuntime.set(runtimeId, scope) + + while (scopeByRuntime.size > MAX_DISMISSED_SESSIONS) { + scopeByRuntime.delete(scopeByRuntime.keys().next().value!) + } + + return scope } function readDismissedPreviewIds(): DismissedPreviewIds { - const value = readJson(dismissedPreviewsKey()) + const value = readJson(DISMISSED_PREVIEWS_KEY) if (!value || typeof value !== 'object' || Array.isArray(value)) { return {} } return Object.fromEntries( - Object.entries(value).flatMap(([sid, ids]) => { - if (!Array.isArray(ids)) { - return [] - } + Object.entries(value) + .slice(-MAX_DISMISSED_SESSIONS) + .flatMap(([sid, ids]) => { + if (!Array.isArray(ids)) { + return [] + } - const strings = ids.filter((id): id is string => typeof id === 'string' && id.length > 0) - return strings.length > 0 ? [[sid, strings]] : [] - }) + if (sid.length > 4096 || !storedOwner(sid)) { + return [] + } + + const strings = ids + .filter((id): id is string => typeof id === 'string' && id.length > 0 && id.length <= 8192) + .slice(-MAX_DISMISSED_TARGETS) + + return strings.length > 0 ? [[sid, strings]] : [] + }) ) } function rememberDismissedPreview(sid: string, id: string): void { const dismissed = readDismissedPreviewIds() - const ids = dismissed[sid] ?? [] + const ids = [...new Set([...(dismissed[sid] ?? []), ...(volatileDismissals.get(sid) ?? [])])] if (ids.includes(id)) { return } - writeJson(dismissedPreviewsKey(), { ...dismissed, [sid]: [...ids, id] }) + const { [sid]: _previous, ...rest } = dismissed + + const next = Object.fromEntries( + [...Object.entries(rest), [sid, [...ids, id].slice(-MAX_DISMISSED_TARGETS)]].slice(-MAX_DISMISSED_SESSIONS) + ) + + // Keep the close effective for this renderer even when storage is unavailable. + volatileDismissals.set(sid, next[sid]) + + while (volatileDismissals.size > MAX_DISMISSED_SESSIONS) { + volatileDismissals.delete(volatileDismissals.keys().next().value!) + } + + if (!sid.startsWith('["runtime",')) { + writeJson(DISMISSED_PREVIEWS_KEY, next) + + if (readDismissedPreviewIds()[sid]?.includes(id)) { + volatileDismissals.delete(sid) + } + } +} + +function rewriteDismissalScopes(rewrite: (scope: string) => string | null): void { + const current = { ...readDismissedPreviewIds(), ...Object.fromEntries(volatileDismissals) } + const next: DismissedPreviewIds = Object.create(null) + + for (const [scope, ids] of Object.entries(current)) { + const target = rewrite(scope) + + if (target) { + next[target] = [...new Set([...(next[target] ?? []), ...ids])].slice(-MAX_DISMISSED_TARGETS) + } + } + + volatileDismissals.clear() + + for (const [scope, ids] of Object.entries(next).slice(-MAX_DISMISSED_SESSIONS)) { + volatileDismissals.set(scope, ids) + } + + writeJson( + DISMISSED_PREVIEWS_KEY, + Object.fromEntries(Object.entries(next).filter(([key]) => !key.startsWith('["runtime",'))) + ) + + for (const [sid, items] of Object.entries($previewStatusBySession.get())) { + writePreviews( + sid, + items.flatMap(item => { + const scope = item.dismissalScope ? rewrite(item.dismissalScope) : undefined + + return scope === null ? [] : [{ ...item, dismissalScope: scope }] + }) + ) + } +} + +function storedOwner(scope: string): [string | null, string, string, string] | null { + try { + const value: unknown = JSON.parse(scope) + + return Array.isArray(value) && + value.length === 4 && + (value[0] === null || typeof value[0] === 'string') && + value.slice(1).every(item => typeof item === 'string') + ? (value as [string | null, string, string, string]) + : null + } catch { + return null + } +} + +export function migratePreviewArtifactsForProfile(from: string, to: string): void { + rewriteDismissalScopes(scope => { + const owner = storedOwner(scope) + + if (!owner || (owner[0] && owner[0] !== 'local')) { + return scope + } + + return JSON.stringify([owner[0], owner[1] === from ? to : owner[1], owner[2] === from ? to : owner[2], owner[3]]) + }) +} + +export function dropPreviewArtifactsForProfile(profile: string, route?: Partial): void { + rewriteDismissalScopes(scope => { + const owner = storedOwner(scope) + + if (!owner) { + return scope + } + + const matches = route + ? owner[1] === route.profile?.trim() && + (!route.connectionId || owner[0] === route.connectionId.trim()) && + (!route.targetProfile || owner[2] === route.targetProfile.trim()) + : (!owner[0] || owner[0] === 'local') && (owner[1] === profile || owner[2] === profile) + + return matches ? null : scope + }) } const writePreviews = (sid: string, items: PreviewArtifact[]) => { @@ -78,7 +241,27 @@ const writePreviews = (sid: string, items: PreviewArtifact[]) => { return } - $previewStatusBySession.set({ ...current, [sid]: items }) + const labelled = items.map(item => { + const name = previewName(item.target) + const peers = items.filter(other => previewName(other.target) === name) + let label = name + + if (peers.length > 1) { + const parts = item.id.split('/') + + for (let depth = 2; depth <= parts.length; depth += 1) { + label = parts.slice(-depth).join('/') + + if (peers.every(other => other.id === item.id || other.id.split('/').slice(-depth).join('/') !== label)) { + break + } + } + } + + return item.label === label ? item : { ...item, label } + }) + + $previewStatusBySession.set({ ...current, [sid]: labelled }) } /** @@ -86,28 +269,56 @@ const writePreviews = (sid: string, items: PreviewArtifact[]) => { * in the list keeps its slot (the tool row re-registers on every render, so this * must not churn the atom or reorder rows). */ -export function recordPreviewArtifact(sid: string, target: string, cwd: string, dismissalSid = sid) { +export function recordPreviewArtifact( + sid: string, + target: string, + cwd: string, + dismissalSid = sid, + newProduction = false +) { const raw = target.trim() + const id = previewArtifactKey(raw, cwd) if (!sid || !raw) { return } + const scope = dismissalKey(sid, dismissalSid) const list = $previewStatusBySession.get()[sid] ?? [] - if (list.some(item => item.id === raw)) { + // Only the live completion handler may re-offer a newly produced file. + // Historical mounts and reconnect replay never grant this intent. + if (newProduction) { + const dismissed = readDismissedPreviewIds() + + if (dismissed[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) { + dismissed[scope] = (dismissed[scope] ?? []).filter(value => value !== id) + volatileDismissals.set( + scope, + (volatileDismissals.get(scope) ?? []).filter(value => value !== id) + ) + writeJson(DISMISSED_PREVIEWS_KEY, dismissed) + } + } + + if (list.some(item => item.id === id)) { return } - if (readDismissedPreviewIds()[dismissalSid]?.includes(raw)) { + if (readDismissedPreviewIds()[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) { return } - writePreviews(sid, [...list, { cwd, id: raw, label: previewName(raw), target: raw }].slice(-MAX_PER_SESSION)) + writePreviews( + sid, + [...list, { cwd, id, label: previewName(raw), target: raw, dismissalScope: scope }].slice(-MAX_PER_SESSION) + ) } export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = sid) { + dismissalKey(sid, dismissalSid) // reconcile an owner refined since the row mounted const list = $previewStatusBySession.get()[sid] + const scope = list?.find(item => item.id === id)?.dismissalScope ?? dismissalKey(sid, dismissalSid) if (list) { writePreviews( @@ -117,7 +328,7 @@ export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = s } if (dismissalSid && id) { - rememberDismissedPreview(dismissalSid, id) + rememberDismissedPreview(scope, id) } } diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index cdcae43981..f64618c328 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -35,6 +35,7 @@ import { stableArray } from '@/lib/stable-array' import { readJson, writeJson } from '@/lib/storage' import type { SessionInfo } from '@/types/hermes' +import { dropPreviewArtifactsForProfile, migratePreviewArtifactsForProfile } from './preview-status' import { $activeGatewayProfile, normalizeProfileKey } from './profile' import { clearAllProviderWaits, clearSessionProviderWait } from './provider-wait' import { @@ -1943,6 +1944,7 @@ export function dropTilesForProfile( } const name = normalizeProfileKey(profile) + dropPreviewArtifactsForProfile(name, route) // Route fields go through the SAME canonicalization as `name` below — a // source-scoped delete must not be defeated by stray whitespace around a // profile name that a non-route delete trims away. @@ -2078,6 +2080,7 @@ export function migrateTilesForProfile(oldProfile: string, newProfile: string): migrateTranscriptTailsForProfile(from, to) migrateRememberedNavigationForProfile(from, to) migrateSessionOwnerHintsForProfile(from, to) + migratePreviewArtifactsForProfile(from, to) } /** ⌘⇧T — reopen the most recently closed tab where it was, then focus it. diff --git a/apps/shared/src/gateway-events.ts b/apps/shared/src/gateway-events.ts index f4cab92169..617790a90a 100644 --- a/apps/shared/src/gateway-events.ts +++ b/apps/shared/src/gateway-events.ts @@ -34,6 +34,8 @@ export type GatewayEventName = keyof GatewayEventMap /** One `event` notification's `params`. */ export interface GatewayEvent { + /** Client-local: recovered/held during reconnect, not fresh user-facing work. */ + replayed?: boolean /** Registry connection whose socket delivered the event (renderer-side tag; * absent for the local/legacy primary path). */ connectionId?: string diff --git a/apps/shared/src/json-rpc-gateway.ts b/apps/shared/src/json-rpc-gateway.ts index fe8f5759b1..676b8ab431 100644 --- a/apps/shared/src/json-rpc-gateway.ts +++ b/apps/shared/src/json-rpc-gateway.ts @@ -522,7 +522,7 @@ export class JsonRpcGatewayClient { continue } - this.dispatchIfNewer(event as GatewayEvent) + this.dispatchIfNewer({ ...event, replayed: true } as GatewayEvent) } } } catch { @@ -584,7 +584,7 @@ export class JsonRpcGatewayClient { for (const parked of hold.values()) { for (const event of parked) { - this.dispatchIfNewer(event) + this.dispatchIfNewer({ ...event, replayed: true }) } } } diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index b32e569827..7ac5fc47b7 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -114,6 +114,8 @@ Explore and preview the working directory without leaving the app — useful for ### Artifacts +Preview links above the composer are session suggestions, not a task-completion checklist. Dismissing one keeps historical tool rows from bringing it back after navigation or reload. A new successful tool completion can offer the file again. Read-only file inspection and failed writes do not create suggestions. Files with the same name show enough directory context to distinguish them; dismissing a suggestion does not delete its file or transcript. Changing a `/goal` does not erase a conversation's artifacts. + When connected to a remote gateway, opening a file artifact downloads it through that gateway, using the artifact’s originating profile and session. Relative paths resolve against the session’s saved working directory; home-relative paths use the gateway’s home, never the Desktop machine’s home. Windows-style relative paths are recognized alongside forward-slash paths, and file URIs retain drive and network-share information for the gateway to interpret. Missing sessions or working directories produce an error rather than selecting a different local file. The **Artifacts** view collects what your sessions generate — **images, files, and links** — into one searchable, browsable gallery. Open it from the sidebar, the command palette (**Artifacts — Browse generated outputs**), or a `nav.artifacts` shortcut you bind yourself. It indexes recent session outputs automatically; every artifact shows which session produced it with a jump back to that chat, and images and files open in a preview with download / open-in-browser / copy actions. From 961ade074c1acc3cb630afc2a56e6eae8f9443e9 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:34:31 +0530 Subject: [PATCH 09/15] refactor(desktop): typed dismissal scopes, one reconcile step, explicit re-offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persisted dismissal scope was a JSON.stringify'd 4-tuple re-parsed by index (owner[0]..owner[3]) and sniffed with a string prefix, and the function that derived it also rewrote persisted scopes as a side effect — so dismissPreviewArtifact called it twice, once for the side effect alone. - DismissalScope interface with one encodeScope/decodeScope pair; isRuntimeScopeKey replaces the two startsWith('["runtime",') probes. - resolveDismissalScope is pure; reconcileDismissalScope owns the refinement rewrite and is called once per record/dismiss. - Profile and target profile are normalizeProfileKey'd when the scope is encoded, matching what dropTilesForProfile/migrateTilesForProfile compare against (a raw '' owner was never dropped). - knownOwnerForSession already falls back to the runtime owner map; the extra runtimeSessionOwner rung could never contribute. - reofferPreviewArtifact replaces the newProduction boolean parameter; the live completion handler is its only caller. - The completion handler derives status + preview target through the new toolPreviewOutcome instead of the full buildToolView (titles, details, stdout parsing) it never renders, and reads the session state once. - recordPreviewArtifact checks the cheap already-listed case before resolving the scope; the list is re-read after reconciling because the rewrite updates listed items' scopes (the refined-append regression). --- .../use-message-stream/gateway-event/tools.ts | 41 +-- .../assistant-ui/tool/fallback-model/index.ts | 11 + apps/desktop/src/store/preview-status.ts | 316 +++++++++++------- 3 files changed, 218 insertions(+), 150 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts index 8d533deedc..2fb7ba7ca7 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/tools.ts @@ -1,10 +1,10 @@ -import { buildToolView, isPreviewableTarget } from '@/components/assistant-ui/tool/fallback-model' +import { isPreviewableTarget, toolPreviewOutcome } from '@/components/assistant-ui/tool/fallback-model' import { reportFirstBuildToolComplete } from '@/components/onboarding-chat/first-build' import { toolCallOwnerMessageId } from '@/lib/chat-messages' import { invalidateSlashCompletions } from '@/lib/slash-completion-cache' import { refreshBackgroundProcesses } from '@/store/composer-status' import { flashPetActivity, setPetActivity } from '@/store/pet' -import { recordPreviewArtifact } from '@/store/preview-status' +import { recordPreviewArtifact, reofferPreviewArtifact } from '@/store/preview-status' import { $sessionStates, storedSessionIdForRuntimeId } from '@/store/session-states' import { pruneDelegateFallbackSubagents, upsertSubagent } from '@/store/subagents' import { reportMcpToolResult } from '@/store/suggestion-providers/repair' @@ -73,32 +73,27 @@ export function handleToolEvent(ctx: GatewayEventContext): boolean { if (sessionId) { flushQueuedDeltas(sessionId) - const pendingProduction = - !event.replayed && Boolean(toolCallOwnerMessageId($sessionStates.get()[sessionId]?.messages ?? [], payload)) + const state = $sessionStates.get()[sessionId] + // Read before the upsert seals the part: only a completion that resolves + // a still-pending call is a fresh production; a duplicate or replayed + // completion may never re-offer a dismissed target. + const pendingProduction = !event.replayed && Boolean(toolCallOwnerMessageId(state?.messages ?? [], payload)) upsertToolCall(sessionId, toTodoPayload(payload) ?? payload, 'complete', event.type, occurredAt) if (!sessionInterrupted(sessionId) && payload?.name && payload.result !== undefined && event.seq !== undefined) { - const view = buildToolView( - { - type: 'tool-call', - toolName: payload.name, - args: payload.args ?? {}, - result: payload.result, - toolResultMetadata: payload, - completedAt: occurredAt - }, - '' - ) + const { previewTarget, status } = toolPreviewOutcome({ + type: 'tool-call', + toolName: payload.name, + args: payload.args ?? {}, + result: payload.result, + toolResultMetadata: payload, + completedAt: occurredAt + }) - if (view.status === 'success' && view.previewTarget && isPreviewableTarget(view.previewTarget)) { - recordPreviewArtifact( - sessionId, - view.previewTarget, - $sessionStates.get()[sessionId]?.cwd ?? '', - storedSessionIdForRuntimeId(sessionId) ?? sessionId, - pendingProduction - ) + if (status === 'success' && previewTarget && isPreviewableTarget(previewTarget)) { + const record = pendingProduction ? reofferPreviewArtifact : recordPreviewArtifact + record(sessionId, previewTarget, state?.cwd ?? '', storedSessionIdForRuntimeId(sessionId) ?? sessionId) } } diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index 468ce141bd..1dd0e161a0 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts @@ -1440,6 +1440,17 @@ function dynamicTitle( return fallback } +/** Status + detected preview target only — for feeds that never render the + * row (the live completion handler) and must not pay for titles/details. */ +export function toolPreviewOutcome(part: ToolPart): { previewTarget: string; status: ToolStatus } { + const resultRecord = toolResultRecord(part) + + return { + previewTarget: toolPreviewTarget(part.toolName, parseMaybeObject(part.args), resultRecord), + status: toolStatus(part, resultRecord) + } +} + export function buildToolView(part: ToolPart, inlineDiff: string): ToolView { const argsRecord = parseMaybeObject(part.args) const resultRecord = toolResultRecord(part) diff --git a/apps/desktop/src/store/preview-status.ts b/apps/desktop/src/store/preview-status.ts index f9d62a97c0..c931cef535 100644 --- a/apps/desktop/src/store/preview-status.ts +++ b/apps/desktop/src/store/preview-status.ts @@ -3,9 +3,10 @@ import { atom } from 'nanostores' import { previewArtifactKey, previewName } from '@/lib/preview-targets' import { readJson, writeJson } from '@/lib/storage' +import { normalizeProfileKey } from './profile' import { ownerLookupSessionRows, resolveComposerSessionKey } from './session' -import type { SessionOwnerRoute } from './session-request-router' -import { knownOwnerForSession, runtimeSessionOwner, storedSessionIdForRuntimeId } from './session-states' +import { isSessionOwnerRoute, type SessionOwnerRoute } from './session-request-router' +import { knownOwnerForSession, storedSessionIdForRuntimeId } from './session-states' /** * Session-scoped feed of previewable artifacts (HTML files, localhost dev URLs) @@ -34,56 +35,120 @@ const MAX_DISMISSED_TARGETS = 64 export const $previewStatusBySession = atom>({}) -interface DismissedPreviewIds { - [session: string]: string[] +/** Durable owner of a dismissal. `connectionId: null` is the legacy bare-profile + * pool (a profile name without a connection tag), not the foreground connection. */ +interface DismissalScope { + connectionId: string | null + profile: string + targetProfile: string + sessionId: string } + +interface DismissedPreviewIds { + [scope: string]: string[] +} + +/** Dismissals that could not (yet) be persisted: runtime-only scopes and + * storage-write failures. Keeps a close effective for this renderer. */ const volatileDismissals = new Map() const scopeByRuntime = new Map() -function dismissalKey(runtimeId: string, storedId: string): string { +const RUNTIME_SCOPE_PREFIX = '["runtime",' + +function runtimeScopeKey(runtimeId: string): string { + return JSON.stringify(['runtime', runtimeId]) +} + +function isRuntimeScopeKey(key: string): boolean { + return key.startsWith(RUNTIME_SCOPE_PREFIX) +} + +function encodeScope(scope: DismissalScope): string { + return JSON.stringify([scope.connectionId, scope.profile, scope.targetProfile, scope.sessionId]) +} + +function decodeScope(key: string): DismissalScope | null { + try { + const value: unknown = JSON.parse(key) + + if ( + !Array.isArray(value) || + value.length !== 4 || + (value[0] !== null && typeof value[0] !== 'string') || + !value.slice(1).every(item => typeof item === 'string') + ) { + return null + } + + const [connectionId, profile, targetProfile, sessionId] = value as [string | null, string, string, string] + + return { connectionId, profile, sessionId, targetProfile } + } catch { + return null + } +} + +function capMap(map: Map): void { + while (map.size > MAX_DISMISSED_SESSIONS) { + map.delete(map.keys().next().value!) + } +} + +/** Pure: the scope key a dismissal for `runtimeId` belongs to right now. */ +function resolveDismissalScope(runtimeId: string, storedId: string): string { // The route's stored selection can advance before the old runtime unmounts. storedId = storedSessionIdForRuntimeId(runtimeId) ?? storedId // Historical rows in background tiles do not belong to the active connection. - const owner = knownOwnerForSession(runtimeId) ?? runtimeSessionOwner(runtimeId) ?? knownOwnerForSession(storedId) - const profile = typeof owner === 'string' ? owner : owner?.targetProfile || owner?.profile + const owner = knownOwnerForSession(runtimeId) ?? knownOwnerForSession(storedId) + + if (!owner) { + return runtimeScopeKey(runtimeId) + } + + const route = isSessionOwnerRoute(owner) ? owner : null + const profile = normalizeProfileKey(route ? route.profile : String(owner)) + const targetProfile = normalizeProfileKey(route?.targetProfile || profile) + const connectionId = route ? route.connectionId : 'local' const rows = ownerLookupSessionRows().filter( - row => - row.profile === profile && - (row.connection_id || 'local') === (typeof owner === 'object' && owner ? owner.connectionId : 'local') + row => normalizeProfileKey(row.profile) === targetProfile && (row.connection_id || 'local') === connectionId ) - const stableId = resolveComposerSessionKey(storedId, rows) ?? storedId - - // Bare profiles name the legacy profile pool, not the foreground connection. - const scope = - owner && typeof owner === 'object' - ? JSON.stringify([owner.connectionId, owner.profile, owner.targetProfile || owner.profile, stableId]) - : typeof owner === 'string' - ? JSON.stringify([null, owner, owner, stableId]) - : JSON.stringify(['runtime', runtimeId]) + return encodeScope({ + connectionId: route ? route.connectionId : null, + profile, + sessionId: resolveComposerSessionKey(storedId, rows) ?? storedId, + targetProfile + }) +} +/** A runtime's owner can be learnt after its rows mounted (unknown → bare + * profile → exact route). Dismissals recorded under the coarser key follow + * the refinement — only for THIS runtime; another profile or connection with + * a coincidentally equal stored id must not inherit them. */ +function reconcileDismissalScope(runtimeId: string, storedId: string): string { + const scope = resolveDismissalScope(runtimeId, storedId) const previous = scopeByRuntime.get(runtimeId) - const before = previous ? storedOwner(previous) : null - const after = storedOwner(scope) - if ( - previous && - previous !== scope && - after && - (previous === JSON.stringify(['runtime', runtimeId]) || - (before && before[0] === null && before[1] === after[1] && before[2] === after[2] && before[3] === after[3])) - ) { - // Only refinement observed on THIS runtime joins keys. Another profile or - // connection with a coincidentally equal stored id must not inherit it. - rewriteDismissalScopes(key => (key === previous ? scope : key)) + if (previous && previous !== scope) { + const before = decodeScope(previous) + const after = decodeScope(scope) + + const refined = + after && + (previous === runtimeScopeKey(runtimeId) || + (before?.connectionId === null && + before.profile === after.profile && + before.targetProfile === after.targetProfile && + before.sessionId === after.sessionId)) + + if (refined) { + rewriteDismissalScopes(key => (key === previous ? scope : key)) + } } scopeByRuntime.set(runtimeId, scope) - - while (scopeByRuntime.size > MAX_DISMISSED_SESSIONS) { - scopeByRuntime.delete(scopeByRuntime.keys().next().value!) - } + capMap(scopeByRuntime) return scope } @@ -98,12 +163,8 @@ function readDismissedPreviewIds(): DismissedPreviewIds { return Object.fromEntries( Object.entries(value) .slice(-MAX_DISMISSED_SESSIONS) - .flatMap(([sid, ids]) => { - if (!Array.isArray(ids)) { - return [] - } - - if (sid.length > 4096 || !storedOwner(sid)) { + .flatMap(([scope, ids]) => { + if (!Array.isArray(ids) || scope.length > 4096 || !decodeScope(scope)) { return [] } @@ -111,41 +172,56 @@ function readDismissedPreviewIds(): DismissedPreviewIds { .filter((id): id is string => typeof id === 'string' && id.length > 0 && id.length <= 8192) .slice(-MAX_DISMISSED_TARGETS) - return strings.length > 0 ? [[sid, strings]] : [] + return strings.length > 0 ? [[scope, strings]] : [] }) ) } -function rememberDismissedPreview(sid: string, id: string): void { +function isDismissed(scope: string, id: string): boolean { + return Boolean(readDismissedPreviewIds()[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) +} + +function rememberDismissedPreview(scope: string, id: string): void { const dismissed = readDismissedPreviewIds() - const ids = [...new Set([...(dismissed[sid] ?? []), ...(volatileDismissals.get(sid) ?? [])])] + const ids = [...new Set([...(dismissed[scope] ?? []), ...(volatileDismissals.get(scope) ?? [])])] if (ids.includes(id)) { return } - const { [sid]: _previous, ...rest } = dismissed + const { [scope]: _previous, ...rest } = dismissed const next = Object.fromEntries( - [...Object.entries(rest), [sid, [...ids, id].slice(-MAX_DISMISSED_TARGETS)]].slice(-MAX_DISMISSED_SESSIONS) + [...Object.entries(rest), [scope, [...ids, id].slice(-MAX_DISMISSED_TARGETS)]].slice(-MAX_DISMISSED_SESSIONS) ) - // Keep the close effective for this renderer even when storage is unavailable. - volatileDismissals.set(sid, next[sid]) + volatileDismissals.set(scope, next[scope]) + capMap(volatileDismissals) - while (volatileDismissals.size > MAX_DISMISSED_SESSIONS) { - volatileDismissals.delete(volatileDismissals.keys().next().value!) - } - - if (!sid.startsWith('["runtime",')) { + if (!isRuntimeScopeKey(scope)) { writeJson(DISMISSED_PREVIEWS_KEY, next) - if (readDismissedPreviewIds()[sid]?.includes(id)) { - volatileDismissals.delete(sid) + if (readDismissedPreviewIds()[scope]?.includes(id)) { + volatileDismissals.delete(scope) } } } +function forgetDismissedPreview(scope: string, id: string): void { + const dismissed = readDismissedPreviewIds() + + if (!dismissed[scope]?.includes(id) && !volatileDismissals.get(scope)?.includes(id)) { + return + } + + dismissed[scope] = (dismissed[scope] ?? []).filter(value => value !== id) + volatileDismissals.set( + scope, + (volatileDismissals.get(scope) ?? []).filter(value => value !== id) + ) + writeJson(DISMISSED_PREVIEWS_KEY, dismissed) +} + function rewriteDismissalScopes(rewrite: (scope: string) => string | null): void { const current = { ...readDismissedPreviewIds(), ...Object.fromEntries(volatileDismissals) } const next: DismissedPreviewIds = Object.create(null) @@ -164,10 +240,7 @@ function rewriteDismissalScopes(rewrite: (scope: string) => string | null): void volatileDismissals.set(scope, ids) } - writeJson( - DISMISSED_PREVIEWS_KEY, - Object.fromEntries(Object.entries(next).filter(([key]) => !key.startsWith('["runtime",'))) - ) + writeJson(DISMISSED_PREVIEWS_KEY, Object.fromEntries(Object.entries(next).filter(([key]) => !isRuntimeScopeKey(key)))) for (const [sid, items] of Object.entries($previewStatusBySession.get())) { writePreviews( @@ -181,48 +254,42 @@ function rewriteDismissalScopes(rewrite: (scope: string) => string | null): void } } -function storedOwner(scope: string): [string | null, string, string, string] | null { - try { - const value: unknown = JSON.parse(scope) - - return Array.isArray(value) && - value.length === 4 && - (value[0] === null || typeof value[0] === 'string') && - value.slice(1).every(item => typeof item === 'string') - ? (value as [string | null, string, string, string]) - : null - } catch { - return null - } -} - export function migratePreviewArtifactsForProfile(from: string, to: string): void { - rewriteDismissalScopes(scope => { - const owner = storedOwner(scope) + rewriteDismissalScopes(key => { + const scope = decodeScope(key) - if (!owner || (owner[0] && owner[0] !== 'local')) { - return scope + if (!scope || (scope.connectionId && scope.connectionId !== 'local')) { + return key } - return JSON.stringify([owner[0], owner[1] === from ? to : owner[1], owner[2] === from ? to : owner[2], owner[3]]) + return encodeScope({ + ...scope, + profile: scope.profile === from ? to : scope.profile, + targetProfile: scope.targetProfile === from ? to : scope.targetProfile + }) }) } export function dropPreviewArtifactsForProfile(profile: string, route?: Partial): void { - rewriteDismissalScopes(scope => { - const owner = storedOwner(scope) + const routeProfile = route?.profile ? normalizeProfileKey(route.profile) : '' + const routeTarget = route?.targetProfile ? normalizeProfileKey(route.targetProfile) : '' + const routeConnection = String(route?.connectionId ?? '').trim() - if (!owner) { - return scope + rewriteDismissalScopes(key => { + const scope = decodeScope(key) + + if (!scope) { + return key } const matches = route - ? owner[1] === route.profile?.trim() && - (!route.connectionId || owner[0] === route.connectionId.trim()) && - (!route.targetProfile || owner[2] === route.targetProfile.trim()) - : (!owner[0] || owner[0] === 'local') && (owner[1] === profile || owner[2] === profile) + ? scope.profile === routeProfile && + (!routeConnection || scope.connectionId === routeConnection) && + (!routeTarget || scope.targetProfile === routeTarget) + : (!scope.connectionId || scope.connectionId === 'local') && + (scope.profile === profile || scope.targetProfile === profile) - return matches ? null : scope + return matches ? null : key }) } @@ -266,59 +333,56 @@ const writePreviews = (sid: string, items: PreviewArtifact[]) => { /** * Record a detected artifact, newest last, capped. Idempotent: a target already - * in the list keeps its slot (the tool row re-registers on every render, so this - * must not churn the atom or reorder rows). + * in the list keeps its slot (the tool row re-registers on every mount, so this + * must not churn the atom or reorder rows). A dismissed target stays hidden — + * historical mounts and reconnect replay never re-offer it. */ -export function recordPreviewArtifact( - sid: string, - target: string, - cwd: string, - dismissalSid = sid, - newProduction = false -) { +export function recordPreviewArtifact(sid: string, target: string, cwd: string, dismissalSid = sid) { const raw = target.trim() - const id = previewArtifactKey(raw, cwd) if (!sid || !raw) { return } - const scope = dismissalKey(sid, dismissalSid) + const id = previewArtifactKey(raw, cwd) + + if ($previewStatusBySession.get()[sid]?.some(item => item.id === id)) { + return + } + + const scope = reconcileDismissalScope(sid, dismissalSid) + + if (isDismissed(scope, id)) { + return + } + + // Re-read: reconciling may have rewritten the listed items' scopes. const list = $previewStatusBySession.get()[sid] ?? [] - // Only the live completion handler may re-offer a newly produced file. - // Historical mounts and reconnect replay never grant this intent. - if (newProduction) { - const dismissed = readDismissedPreviewIds() - - if (dismissed[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) { - dismissed[scope] = (dismissed[scope] ?? []).filter(value => value !== id) - volatileDismissals.set( - scope, - (volatileDismissals.get(scope) ?? []).filter(value => value !== id) - ) - writeJson(DISMISSED_PREVIEWS_KEY, dismissed) - } - } - - if (list.some(item => item.id === id)) { - return - } - - if (readDismissedPreviewIds()[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) { - return - } - writePreviews( sid, [...list, { cwd, id, label: previewName(raw), target: raw, dismissalScope: scope }].slice(-MAX_PER_SESSION) ) } +/** A genuinely new successful production of a target the user dismissed + * earlier may offer it again. Only the live completion handler has that + * intent; see `recordPreviewArtifact` for every other feed. */ +export function reofferPreviewArtifact(sid: string, target: string, cwd: string, dismissalSid = sid) { + const raw = target.trim() + + if (!sid || !raw) { + return + } + + forgetDismissedPreview(reconcileDismissalScope(sid, dismissalSid), previewArtifactKey(raw, cwd)) + recordPreviewArtifact(sid, target, cwd, dismissalSid) +} + export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = sid) { - dismissalKey(sid, dismissalSid) // reconcile an owner refined since the row mounted + const current = reconcileDismissalScope(sid, dismissalSid) const list = $previewStatusBySession.get()[sid] - const scope = list?.find(item => item.id === id)?.dismissalScope ?? dismissalKey(sid, dismissalSid) + const scope = list?.find(item => item.id === id)?.dismissalScope ?? current if (list) { writePreviews( @@ -327,9 +391,7 @@ export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = s ) } - if (dismissalSid && id) { - rememberDismissedPreview(scope, id) - } + rememberDismissedPreview(scope, id) } export function clearPreviewArtifacts(sid: string) { From d00b511da178d84ce0714e11ae091da0d3bb3c61 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:34:31 +0530 Subject: [PATCH 10/15] refactor(desktop): tool row preview effect gates on view status only toolStatus never reports success without a result, so the isPending and result === undefined guards (and their deps) were redundant. The optional chain on $storedId existed only for a test SessionView stub that omitted the non-optional atom; the stub now declares it. --- .../tool/fallback-preview-scope.test.tsx | 1 + .../components/assistant-ui/tool/fallback.tsx | 23 +++---------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx index c853f565bf..fa526f3c2f 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-preview-scope.test.tsx @@ -27,6 +27,7 @@ function tileView(): SessionView { $cwd: atom('/tile/work'), $messages: atom(messages), $runtimeId: atom(TILE_ID), + $storedId: atom(null), kind: 'tile' } } diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 54d98fa79d..875e6b0b3b 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -427,13 +427,7 @@ function ToolEntry({ part }: ToolEntryProps) { } = useSessionView() useEffect(() => { - if ( - isPending || - result === undefined || - view.status !== 'success' || - !previewTarget || - !isPreviewableTarget(previewTarget) - ) { + if (view.status !== 'success' || !previewTarget || !isPreviewableTarget(previewTarget)) { return } @@ -441,24 +435,13 @@ function ToolEntry({ part }: ToolEntryProps) { // target appears, and subscribing re-rendered every tool row on any session // or cwd change. const sessionId = $sessionRuntimeId.get() - const dismissalSessionId = $sessionStoredId?.get() ?? sessionId // A route switch can paint the previous assistant row while these atoms // already describe the next chat. Only that chat's own messages may feed it. if (sessionId && $sessionMessages.get().some(message => message.id === messageId)) { - recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '', dismissalSessionId ?? '') + recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '', $sessionStoredId.get() ?? sessionId) } - }, [ - $sessionCwd, - $sessionRuntimeId, - $sessionStoredId, - $sessionMessages, - isPending, - messageId, - previewTarget, - result, - view.status - ]) + }, [$sessionCwd, $sessionRuntimeId, $sessionStoredId, $sessionMessages, messageId, previewTarget, view.status]) const detailSections = useMemo(() => { if (!view.detail) { From d28762d88d388125088c7448901c909655c19210 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:34:59 +0530 Subject: [PATCH 11/15] test(desktop): trim overlapping preview-dismissal cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plain dismiss→re-record suppression was asserted twice (once with, once without an owner refinement); the refinement case is the stricter contract and stays. The storage-failure and module-reload invariants were bundled in one test; they are unrelated and now fail independently. --- .../src/store/preview-status-lifecycle.test.ts | 4 +++- apps/desktop/src/store/preview-status.test.ts | 15 --------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/apps/desktop/src/store/preview-status-lifecycle.test.ts b/apps/desktop/src/store/preview-status-lifecycle.test.ts index a482a72033..8979f02077 100644 --- a/apps/desktop/src/store/preview-status-lifecycle.test.ts +++ b/apps/desktop/src/store/preview-status-lifecycle.test.ts @@ -37,7 +37,7 @@ it('moves local dismissals on rename and removes only the deleted owner', () => expect($previewStatusBySession.get()['rename-remote']).toBeUndefined() }) -it('honors memory-only closes on storage failure and durable closes after module reload', async () => { +it('keeps a close effective in memory when storage writes fail', () => { recordSessionEventScope({ session_id: 'quota', connectionId: 'local', profile: 'quota' }) recordPreviewArtifact('quota', '/work/quota.html', '/work', 'quota-stored') @@ -50,7 +50,9 @@ it('honors memory-only closes on storage failure and durable closes after module recordPreviewArtifact('quota', '/work/quota.html', '/work', 'quota-stored') expect($previewStatusBySession.get().quota).toBeUndefined() write.mockRestore() +}) +it('honors a durable close after module reload', async () => { recordSessionEventScope({ session_id: 'before-reload', connectionId: 'local', profile: 'persist' }) recordPreviewArtifact('before-reload', '/work/saved.html', '/work', 'persist-stored') dismissPreviewArtifact('before-reload', '/work/saved.html', 'persist-stored') diff --git a/apps/desktop/src/store/preview-status.test.ts b/apps/desktop/src/store/preview-status.test.ts index 41c7622899..97adc0858a 100644 --- a/apps/desktop/src/store/preview-status.test.ts +++ b/apps/desktop/src/store/preview-status.test.ts @@ -45,21 +45,6 @@ describe('recordPreviewArtifact', () => { expect($previewStatusBySession.get().s1).toBeUndefined() }) - it('suppresses a dismissed historical target when the row mounts again', () => { - for (const session_id of ['runtime-a', 'runtime-b']) { - recordSessionEventScope({ session_id, connectionId: 'owner', profile: 'default' }) - } - - recordPreviewArtifact('runtime-a', '/a/index.html', '/work', 'stored-a') - dismissPreviewArtifact('runtime-a', '/a/index.html', 'stored-a') - $previewStatusBySession.set({}) - - recordPreviewArtifact('runtime-b', '/a/index.html', '/work', 'stored-a') - - expect($previewStatusBySession.get()['runtime-b']).toBeUndefined() - expect(window.localStorage.getItem('hermes.desktop.previewDismissals.v1')).toContain('stored-a') - }) - it('keeps dismissal with the session owner across foreground changes and runtime rebinds', () => { const record = (runtime: string, connectionId: string, profile: string) => { recordSessionEventScope({ session_id: runtime, connectionId, profile }) From c1488ac947c9bc33fd65ec464548dc9d8edd6122 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:43:31 +0530 Subject: [PATCH 12/15] refactor(desktop): runtime dismissal keys cannot collide with owner scopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runtime-only scopes were JSON-encoded 2-tuples living in the same key space as the 4-tuple owner scopes, detected by a string prefix — an owner route whose connection id was literally "runtime" would encode to a key that matched and never be persisted. The runtime key is now a non-JSON string. Remember and forget share one updateDismissed step, so forget also skips storage for runtime scopes and drops empty arrays instead of writing them. Owner narrowing goes through isSessionOwnerRoute instead of a String() cast; re-offer resolves the scope once and appends through the same tail as record. --- apps/desktop/src/store/preview-status.ts | 106 +++++++++++++---------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/apps/desktop/src/store/preview-status.ts b/apps/desktop/src/store/preview-status.ts index c931cef535..9c44203e7e 100644 --- a/apps/desktop/src/store/preview-status.ts +++ b/apps/desktop/src/store/preview-status.ts @@ -53,10 +53,11 @@ interface DismissedPreviewIds { const volatileDismissals = new Map() const scopeByRuntime = new Map() -const RUNTIME_SCOPE_PREFIX = '["runtime",' +/** Not JSON on purpose: it can never collide with an encoded owner scope. */ +const RUNTIME_SCOPE_PREFIX = 'runtime:' function runtimeScopeKey(runtimeId: string): string { - return JSON.stringify(['runtime', runtimeId]) + return RUNTIME_SCOPE_PREFIX + runtimeId } function isRuntimeScopeKey(key: string): boolean { @@ -94,7 +95,8 @@ function capMap(map: Map): void { } } -/** Pure: the scope key a dismissal for `runtimeId` belongs to right now. */ +/** The scope key a dismissal for `runtimeId` belongs to right now. Reads the + * owner stores but never touches `scopeByRuntime`; see reconcileDismissalScope. */ function resolveDismissalScope(runtimeId: string, storedId: string): string { // The route's stored selection can advance before the old runtime unmounts. storedId = storedSessionIdForRuntimeId(runtimeId) ?? storedId @@ -106,16 +108,18 @@ function resolveDismissalScope(runtimeId: string, storedId: string): string { } const route = isSessionOwnerRoute(owner) ? owner : null - const profile = normalizeProfileKey(route ? route.profile : String(owner)) + const profile = normalizeProfileKey(isSessionOwnerRoute(owner) ? owner.profile : owner) const targetProfile = normalizeProfileKey(route?.targetProfile || profile) - const connectionId = route ? route.connectionId : 'local' + // A bare profile names the legacy pool (`null`), which rows tag as local. + const connectionId = route?.connectionId ?? null const rows = ownerLookupSessionRows().filter( - row => normalizeProfileKey(row.profile) === targetProfile && (row.connection_id || 'local') === connectionId + row => + normalizeProfileKey(row.profile) === targetProfile && (row.connection_id || 'local') === (connectionId ?? 'local') ) return encodeScope({ - connectionId: route ? route.connectionId : null, + connectionId, profile, sessionId: resolveComposerSessionKey(storedId, rows) ?? storedId, targetProfile @@ -181,45 +185,46 @@ function isDismissed(scope: string, id: string): boolean { return Boolean(readDismissedPreviewIds()[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) } -function rememberDismissedPreview(scope: string, id: string): void { +/** Apply `update` to one scope's dismissed ids in both the persisted map and + * the renderer-local copy. Runtime-only scopes never reach storage; a failed + * storage write leaves the local copy in force for this renderer. */ +function updateDismissed(scope: string, update: (ids: string[]) => string[]): void { const dismissed = readDismissedPreviewIds() - const ids = [...new Set([...(dismissed[scope] ?? []), ...(volatileDismissals.get(scope) ?? [])])] - - if (ids.includes(id)) { - return - } - + const ids = update([...new Set([...(dismissed[scope] ?? []), ...(volatileDismissals.get(scope) ?? [])])]) const { [scope]: _previous, ...rest } = dismissed const next = Object.fromEntries( - [...Object.entries(rest), [scope, [...ids, id].slice(-MAX_DISMISSED_TARGETS)]].slice(-MAX_DISMISSED_SESSIONS) + [...Object.entries(rest), ...(ids.length > 0 ? [[scope, ids.slice(-MAX_DISMISSED_TARGETS)]] : [])].slice( + -MAX_DISMISSED_SESSIONS + ) ) - volatileDismissals.set(scope, next[scope]) - capMap(volatileDismissals) + if (ids.length > 0) { + volatileDismissals.set(scope, next[scope]) + capMap(volatileDismissals) + } else { + volatileDismissals.delete(scope) + } if (!isRuntimeScopeKey(scope)) { writeJson(DISMISSED_PREVIEWS_KEY, next) - if (readDismissedPreviewIds()[scope]?.includes(id)) { + if (ids.length > 0 && readDismissedPreviewIds()[scope]?.includes(ids[ids.length - 1])) { volatileDismissals.delete(scope) } } } -function forgetDismissedPreview(scope: string, id: string): void { - const dismissed = readDismissedPreviewIds() - - if (!dismissed[scope]?.includes(id) && !volatileDismissals.get(scope)?.includes(id)) { - return +function rememberDismissedPreview(scope: string, id: string): void { + if (!isDismissed(scope, id)) { + updateDismissed(scope, ids => [...ids, id]) } +} - dismissed[scope] = (dismissed[scope] ?? []).filter(value => value !== id) - volatileDismissals.set( - scope, - (volatileDismissals.get(scope) ?? []).filter(value => value !== id) - ) - writeJson(DISMISSED_PREVIEWS_KEY, dismissed) +function forgetDismissedPreview(scope: string, id: string): void { + if (isDismissed(scope, id)) { + updateDismissed(scope, ids => ids.filter(value => value !== id)) + } } function rewriteDismissalScopes(rewrite: (scope: string) => string | null): void { @@ -271,9 +276,9 @@ export function migratePreviewArtifactsForProfile(from: string, to: string): voi } export function dropPreviewArtifactsForProfile(profile: string, route?: Partial): void { - const routeProfile = route?.profile ? normalizeProfileKey(route.profile) : '' + const routeProfile = normalizeProfileKey(route?.profile) const routeTarget = route?.targetProfile ? normalizeProfileKey(route.targetProfile) : '' - const routeConnection = String(route?.connectionId ?? '').trim() + const routeConnection = (route?.connectionId ?? '').trim() rewriteDismissalScopes(key => { const scope = decodeScope(key) @@ -331,6 +336,24 @@ const writePreviews = (sid: string, items: PreviewArtifact[]) => { $previewStatusBySession.set({ ...current, [sid]: labelled }) } +function appendPreviewArtifact(sid: string, raw: string, cwd: string, id: string, scope: string): void { + if (isDismissed(scope, id)) { + return + } + + // Read after reconciling: the refinement rewrite updates listed items' scopes. + const list = $previewStatusBySession.get()[sid] ?? [] + + if (list.some(item => item.id === id)) { + return + } + + writePreviews( + sid, + [...list, { cwd, id, label: previewName(raw), target: raw, dismissalScope: scope }].slice(-MAX_PER_SESSION) + ) +} + /** * Record a detected artifact, newest last, capped. Idempotent: a target already * in the list keeps its slot (the tool row re-registers on every mount, so this @@ -350,19 +373,7 @@ export function recordPreviewArtifact(sid: string, target: string, cwd: string, return } - const scope = reconcileDismissalScope(sid, dismissalSid) - - if (isDismissed(scope, id)) { - return - } - - // Re-read: reconciling may have rewritten the listed items' scopes. - const list = $previewStatusBySession.get()[sid] ?? [] - - writePreviews( - sid, - [...list, { cwd, id, label: previewName(raw), target: raw, dismissalScope: scope }].slice(-MAX_PER_SESSION) - ) + appendPreviewArtifact(sid, raw, cwd, id, reconcileDismissalScope(sid, dismissalSid)) } /** A genuinely new successful production of a target the user dismissed @@ -375,8 +386,11 @@ export function reofferPreviewArtifact(sid: string, target: string, cwd: string, return } - forgetDismissedPreview(reconcileDismissalScope(sid, dismissalSid), previewArtifactKey(raw, cwd)) - recordPreviewArtifact(sid, target, cwd, dismissalSid) + const id = previewArtifactKey(raw, cwd) + const scope = reconcileDismissalScope(sid, dismissalSid) + + forgetDismissedPreview(scope, id) + appendPreviewArtifact(sid, raw, cwd, id, scope) } export function dismissPreviewArtifact(sid: string, id: string, dismissalSid = sid) { From 439ebe0ae9f1caa10ee5ca9620a705504d364a72 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:09:20 -0700 Subject: [PATCH 13/15] fix(tests): stop the code_kernel reader-thread leak that OOM-killed test workers; cap worker heap tests/tools/test_local_env_blocklist.py::TestPythonpathSelectiveStrip:: test_execute_code_composition_strips_inherited_hermes_entries hands code_kernel a MagicMock process whose stdout/stderr only fake read(); the kernel drains with read1(), which on a bare MagicMock never returns EOF. _stdout_reader died on `buf += chunk`, but _stderr_reader's `while chunk := stderr.read1(4096)` spun forever appending mocks (each call growing mock_calls) in a daemon thread that outlived the test: ~1 GB/min until the kernel killed the worker. Five OOM incidents on this file (08-30, 09-13, 09-14, 09-16, 09-19), always blamed on whichever test ran next. The fake now returns EOF from read1() as well. Runner guardrails so the next runaway is a traceback, not a swap storm: - each pytest worker runs under RLIMIT_DATA (8 GiB, Linux; HERMES_TEST_WORKER_MEM_GB, 0 = off). RLIMIT_AS is avoided on purpose: browsers spawned by tests reserve huge address space. - a worker killed by signal or the file timeout is never --file-retries relaunched; a runaway relaunched while the first tree is still being reaped doubled the damage on 09-16. Live: the file went from 20 min / 20 GB to 5.6 s / 140 MB. An allocate-forever probe dies with MemoryError in 5 s; a SIGKILL'd worker launches once on this runner, twice on base. --- AGENTS.md | 4 +++- scripts/run_tests.sh | 2 +- scripts/run_tests_parallel.py | 31 +++++++++++++++++++++++++ tests/tools/test_local_env_blocklist.py | 5 ++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 28bd535616..3ea098b15e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -340,7 +340,9 @@ scripts/run_tests.sh -v --tb=long # pytest flags pass thro ``` - **Flake policy:** a failing FILE is retried once in a fresh subprocess (`--file-retries`; - `HERMES_TEST_FILE_RETRIES=0` disables). Pass-on-retry is green but printed under `⚠ FLAKY` + `HERMES_TEST_FILE_RETRIES=0` disables); a worker killed by signal or the file timeout is never + retried. Each worker runs under an 8 GiB heap cap on Linux (`HERMES_TEST_WORKER_MEM_GB`, 0 + disables) so a runaway loop dies with a `MemoryError` traceback instead of swapping the host. Pass-on-retry is green but printed under `⚠ FLAKY` with both outputs — a bug to fix, not noise. Timing tests must not assume a quiet runner: wall-clock bounds ≥ 2s, event-based sync, no `assert not _wait_until(...)` races. - **Placement mirrors the source tree.** A test lives in `tests//` (`tests/hermes_cli/`, diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 77eedc6d1c..d2e9aedb49 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -143,7 +143,7 @@ done # credential can leak" property stays auditable at a glance. TEST_ENV=() for _test_var in HERMES_TEST_IMAGE HERMES_TEST_WORKERS HERMES_TEST_PATHS \ - HERMES_TEST_FILE_TIMEOUT HERMES_TEST_FILE_RETRIES HERMES_TEST_SLICE; do + HERMES_TEST_FILE_TIMEOUT HERMES_TEST_FILE_RETRIES HERMES_TEST_WORKER_MEM_GB HERMES_TEST_SLICE; do if [ -n "${!_test_var:-}" ]; then TEST_ENV+=("$_test_var=${!_test_var}") fi diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index af1c08f626..5acf053d6b 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -117,6 +117,13 @@ _DEFAULT_FILE_TIMEOUT_SECONDS = 300.0 # Set to 0 to disable (env: HERMES_TEST_FILE_RETRIES). _DEFAULT_FILE_RETRIES = 1 +# Per-worker heap cap in GiB (Linux RLIMIT_DATA: brk + private anonymous mmap; inherited by the +# worker's children). A runaway allocation loop then dies with a MemoryError traceback in seconds +# instead of swapping the host for 20 GB with no stack (five OOM incidents from one leaking test +# thread). RLIMIT_AS is deliberately NOT used: browsers spawned by tests reserve huge address +# space. Set to 0 to disable (env: HERMES_TEST_WORKER_MEM_GB). +_DEFAULT_WORKER_MEM_GB = 8.0 + # Duration cache: maps relative file paths to last-observed subprocess # wall-clock seconds. Used by ``--slice`` to distribute files across # CI jobs by estimated total time, so no one job gets all the slow files. @@ -414,6 +421,10 @@ def _run_one_file( file, pytest_args, repo_root, file_timeout ) attempt = 0 + # A worker killed by signal (OOM, SIGKILL) or the file timeout is a runaway, not a flake: + # relaunching it doubles the damage while the first tree is still being reaped. + if rc < 0 or rc == 124: + retries = 0 while rc != 0 and attempt < retries: attempt += 1 first_output = output @@ -441,6 +452,25 @@ _FLAKY_RESULTS: List[Tuple[Path, str]] = [] _flaky_lock = threading.Lock() +def _worker_memory_cap(): + """``preexec_fn`` applying the per-worker heap cap, or None when disabled / unsupported.""" + if sys.platform != "linux": + return None + try: + gib = float(os.environ.get("HERMES_TEST_WORKER_MEM_GB", _DEFAULT_WORKER_MEM_GB)) + except ValueError: + gib = _DEFAULT_WORKER_MEM_GB + if gib <= 0: + return None + limit = int(gib * (1 << 30)) + + def _apply(): + import resource + resource.setrlimit(resource.RLIMIT_DATA, (limit, limit)) + + return _apply + + def _run_one_file_once( file: Path, pytest_args: List[str], @@ -482,6 +512,7 @@ def _run_one_file_once( stderr=subprocess.STDOUT, text=True, encoding="utf-8", errors="replace", env=env, + preexec_fn=_worker_memory_cap(), # POSIX: place the child at the head of its own process group so # _kill_tree can SIGKILL the group atomically. # Windows: this maps to CREATE_NEW_PROCESS_GROUP in CPython 3.12+; diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 3f6b062c51..4fefee7780 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -976,8 +976,13 @@ class TestPythonpathSelectiveStrip: captured["env"] = kwargs.get("env", {}) captured["staging"] = os.path.dirname(cmd[1]) proc = MagicMock() + # The kernel's reader threads drain with read1(); a bare MagicMock never returns + # EOF there, so the stderr thread spins forever appending mocks (a 1 GB/min leak + # that outlived the test and OOM-killed the worker five times). proc.stdout.read.return_value = b"" + proc.stdout.read1.return_value = b"" proc.stderr.read.return_value = b"" + proc.stderr.read1.return_value = b"" proc.wait.return_value = 0 proc.returncode = 0 proc.poll.return_value = 0 From 6159bf4d87a32de72869e0b479606ddd2b9977a1 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 20 Sep 2026 08:53:58 -0700 Subject: [PATCH 14/15] runner: drop the RLIMIT_DATA worker cap On the CI runner the pillow-heif HEIF encode in tests/tools/test_image_source.py hangs under the cap (2/2 runs, 36/40 then 300 s timeout; 40/40 in 26 s on main). The wheel's encoder spins on a failed allocation instead of erroring, so a heap cap on C code trades an OOM for a hang. Keep the leak fix and the no-relaunch-on-kill rule. --- AGENTS.md | 3 +-- scripts/run_tests.sh | 2 +- scripts/run_tests_parallel.py | 27 --------------------------- 3 files changed, 2 insertions(+), 30 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3ea098b15e..1a9edd8af8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -341,8 +341,7 @@ scripts/run_tests.sh -v --tb=long # pytest flags pass thro - **Flake policy:** a failing FILE is retried once in a fresh subprocess (`--file-retries`; `HERMES_TEST_FILE_RETRIES=0` disables); a worker killed by signal or the file timeout is never - retried. Each worker runs under an 8 GiB heap cap on Linux (`HERMES_TEST_WORKER_MEM_GB`, 0 - disables) so a runaway loop dies with a `MemoryError` traceback instead of swapping the host. Pass-on-retry is green but printed under `⚠ FLAKY` + retried (relaunching a runaway doubles the damage). Pass-on-retry is green but printed under `⚠ FLAKY` with both outputs — a bug to fix, not noise. Timing tests must not assume a quiet runner: wall-clock bounds ≥ 2s, event-based sync, no `assert not _wait_until(...)` races. - **Placement mirrors the source tree.** A test lives in `tests//` (`tests/hermes_cli/`, diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index d2e9aedb49..77eedc6d1c 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -143,7 +143,7 @@ done # credential can leak" property stays auditable at a glance. TEST_ENV=() for _test_var in HERMES_TEST_IMAGE HERMES_TEST_WORKERS HERMES_TEST_PATHS \ - HERMES_TEST_FILE_TIMEOUT HERMES_TEST_FILE_RETRIES HERMES_TEST_WORKER_MEM_GB HERMES_TEST_SLICE; do + HERMES_TEST_FILE_TIMEOUT HERMES_TEST_FILE_RETRIES HERMES_TEST_SLICE; do if [ -n "${!_test_var:-}" ]; then TEST_ENV+=("$_test_var=${!_test_var}") fi diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 5acf053d6b..9f7622daaf 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -117,13 +117,6 @@ _DEFAULT_FILE_TIMEOUT_SECONDS = 300.0 # Set to 0 to disable (env: HERMES_TEST_FILE_RETRIES). _DEFAULT_FILE_RETRIES = 1 -# Per-worker heap cap in GiB (Linux RLIMIT_DATA: brk + private anonymous mmap; inherited by the -# worker's children). A runaway allocation loop then dies with a MemoryError traceback in seconds -# instead of swapping the host for 20 GB with no stack (five OOM incidents from one leaking test -# thread). RLIMIT_AS is deliberately NOT used: browsers spawned by tests reserve huge address -# space. Set to 0 to disable (env: HERMES_TEST_WORKER_MEM_GB). -_DEFAULT_WORKER_MEM_GB = 8.0 - # Duration cache: maps relative file paths to last-observed subprocess # wall-clock seconds. Used by ``--slice`` to distribute files across # CI jobs by estimated total time, so no one job gets all the slow files. @@ -452,25 +445,6 @@ _FLAKY_RESULTS: List[Tuple[Path, str]] = [] _flaky_lock = threading.Lock() -def _worker_memory_cap(): - """``preexec_fn`` applying the per-worker heap cap, or None when disabled / unsupported.""" - if sys.platform != "linux": - return None - try: - gib = float(os.environ.get("HERMES_TEST_WORKER_MEM_GB", _DEFAULT_WORKER_MEM_GB)) - except ValueError: - gib = _DEFAULT_WORKER_MEM_GB - if gib <= 0: - return None - limit = int(gib * (1 << 30)) - - def _apply(): - import resource - resource.setrlimit(resource.RLIMIT_DATA, (limit, limit)) - - return _apply - - def _run_one_file_once( file: Path, pytest_args: List[str], @@ -512,7 +486,6 @@ def _run_one_file_once( stderr=subprocess.STDOUT, text=True, encoding="utf-8", errors="replace", env=env, - preexec_fn=_worker_memory_cap(), # POSIX: place the child at the head of its own process group so # _kill_tree can SIGKILL the group atomically. # Windows: this maps to CREATE_NEW_PROCESS_GROUP in CPython 3.12+; From 0469740ab33fd02a4f55a6ea11d81df04ea646a5 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:15:42 -0700 Subject: [PATCH 15/15] feat(cron): jobs follow the main agent model at fire time; `pinned` locks it on request An unpinned cron job used to snapshot the global provider/model at creation and treat that snapshot as its effective pin (#44585), so `hermes model` / `/model` never moved the fleet and `hermes cron resnap` existed to catch jobs up. New ruling: jobs run on whatever the main agent model is when they fire. Resolution is per-job pin > cron.model / cron.model_provider (the cron fleet default) > model.default. `pinned` replaces the implicit snapshot with an explicit lock: create/update with pinned=true writes the CURRENT main provider+model onto the job as an ordinary per-job pin; pinned=false releases both. The cronjob tool exposes it (schema: only when the user asks; it can only lock the main model, never point spend at a different one) and reports `pinned` per job; the CLI gets `--pin` / `--unpin`. Legacy records that still carry *_snapshot keys follow the main model. Removed with the snapshot: `hermes cron resnap`, the tool's resnap action + `all` param, the "N unpinned jobs keep running on ..." notice in `hermes model` / `hermes config set` / the dashboard model assignment, and the Desktop cron-model-impact card (setMainModelAssignment keeps the expensive-model confirm flow in store/model-assignment.ts). Live A/B (real store + run_job against a temp HERMES_HOME): main-model X -> Y, unpinned job fires on X before, Y after; pinned job stays on X; unpin -> Y; legacy snapshot record -> Y. --- COMPAT_MANIFEST.md | 2 - .../src/app/settings/model-settings.tsx | 2 +- apps/desktop/src/i18n/ar.ts | 20 +- apps/desktop/src/i18n/en.ts | 20 +- apps/desktop/src/i18n/ja.ts | 20 +- apps/desktop/src/i18n/ru.ts | 12 +- apps/desktop/src/i18n/types.ts | 19 +- apps/desktop/src/i18n/zh-hant.ts | 19 +- apps/desktop/src/i18n/zh.ts | 19 +- apps/desktop/src/lib/connection-scoped.ts | 3 +- .../src/store/cron-model-impact-scope.test.ts | 50 --- .../src/store/cron-model-impact-scope.ts | 63 ---- .../src/store/cron-model-impact.test.ts | 274 ----------------- apps/desktop/src/store/cron-model-impact.ts | 263 ---------------- apps/desktop/src/store/model-assignment.ts | 83 +++++ apps/desktop/src/store/onboarding.ts | 2 +- apps/desktop/src/store/profile.ts | 2 - apps/desktop/src/store/session.ts | 2 - apps/desktop/src/types/hermes.ts | 17 - cli.py | 4 - compat_manifest.json | 12 - cron/jobs.py | 189 +++--------- cron/scheduler.py | 43 +-- hermes_cli/config.py | 167 ---------- hermes_cli/config_defaults.py | 5 +- hermes_cli/cron.py | 36 +-- hermes_cli/model_switch.py | 4 +- hermes_cli/subcommands/cron.py | 29 +- hermes_cli/web_server.py | 2 - hermes_cli/web_server_config.py | 18 -- tests/cron/test_cron_provider_pin.py | 291 ++++-------------- tests/cron/test_cron_reasoning_effort.py | 8 - tests/cron/test_scheduler.py | 5 +- .../hermes_cli/test_cli_save_config_value.py | 16 - tests/hermes_cli/test_cron_model_impact.py | 177 ----------- tests/hermes_cli/test_set_config_value.py | 41 --- .../test_web_server_cron_profiles.py | 96 ------ .../test_web_server_profile_unification.py | 115 ------- tests/tools/test_cronjob_tools.py | 39 --- tools/cronjob_job_args.py | 2 + tools/cronjob_tools.py | 68 +--- website/docs/user-guide/features/cron.md | 31 +- 42 files changed, 292 insertions(+), 1998 deletions(-) delete mode 100644 apps/desktop/src/store/cron-model-impact-scope.test.ts delete mode 100644 apps/desktop/src/store/cron-model-impact-scope.ts delete mode 100644 apps/desktop/src/store/cron-model-impact.test.ts delete mode 100644 apps/desktop/src/store/cron-model-impact.ts create mode 100644 apps/desktop/src/store/model-assignment.ts delete mode 100644 tests/hermes_cli/test_cron_model_impact.py diff --git a/COMPAT_MANIFEST.md b/COMPAT_MANIFEST.md index 992a02050d..79ef74f59d 100644 --- a/COMPAT_MANIFEST.md +++ b/COMPAT_MANIFEST.md @@ -1882,7 +1882,6 @@ to the public equivalent or the new module. Test monkeypatch seams are likewise | `auth_mcp_server` | moved-lazy | `hermes_cli.web_routers.mcp` | | `base64` | import | `base64` | | `binascii` | import | `binascii` | -| `build_cron_model_impact` | moved-lazy | `hermes_cli.config` | | `bulk_delete_sessions_endpoint` | moved-lazy | `hermes_cli.web_routers.sessions` | | `cancel_oauth_session` | moved-lazy | `hermes_cli.web_routers.oauth` | | `cancel_telegram_onboarding` | moved-lazy | `hermes_cli.web_routers.messaging` | @@ -2086,7 +2085,6 @@ to the public equivalent or the new module. Test monkeypatch seams are likewise | `replace_mcp_servers` | moved-lazy | `hermes_cli.web_routers.mcp` | | `rescan_dashboard_plugins` | moved-lazy | `hermes_cli.web_routers.dashboard_ui` | | `reset_memory` | moved-lazy | `hermes_cli.web_routers.ops` | -| `resolve_cron_model_drift_defaults` | moved-lazy | `hermes_cli.config` | | `resolve_gateway_liveness` | moved-lazy | `gateway.status` | | `restart_gateway` | moved-lazy | `hermes_cli.web_routers.actions` | | `resume_cron_job` | moved-lazy | `hermes_cli.web_routers.cron` | diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index 901c415d52..6ac73cd9ec 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -30,7 +30,7 @@ import { isCodeSkewRestartRequired } from '@/lib/code-skew-error' import { AlertTriangle, Cpu, Loader2 } from '@/lib/icons' import { isSubmitEnter } from '@/lib/ime' import { cn } from '@/lib/utils' -import { setMainModelAssignment } from '@/store/cron-model-impact' +import { setMainModelAssignment } from '@/store/model-assignment' import { notifyError, readableError } from '@/store/notifications' import { startManualLocalEndpoint, startManualOnboarding, startManualProviderOAuth } from '@/store/onboarding' diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 374187684e..95e4f98287 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1779,20 +1779,16 @@ export const ar = defineLocale({ failedCreate: 'فشل الإنشاء', failedRename: 'فشل إعادة التسمية' }, + modelAssignment: { + saveFailed: 'لم يحفظ Hermes تغيير النموذج هذا.', + confirmTitle: 'تحذير اختيار النموذج', + confirmDetail: 'أكّد فقط إذا كنت تقبل هذه المقايضة.', + confirmAction: 'تأكيد', + declined: 'أُلغي تغيير النموذج — رفضت تحذير طبقة تدريب البيانات.' + }, + cron: { close: 'إغلاق', - modelImpact: { - title: 'تبقى المهام المجدولة على نموذجها الأصلي', - message: count => - `${count} من المهام المجدولة غير المثبتة ستواصل العمل على النموذج الذي أُنشئت به. ثبّتها أو اضبط cron.model لنقلها.`, - detailMore: (names, remaining) => `${names} و${remaining} أخرى`, - review: 'مراجعة المهام المجدولة', - saveFailed: 'لم يحفظ Hermes تغيير النموذج هذا.', - confirmTitle: 'تحذير اختيار النموذج', - confirmDetail: 'أكّد فقط إذا كنت تقبل هذه المقايضة.', - confirmAction: 'تأكيد', - declined: 'أُلغي تغيير النموذج — رفضت تحذير طبقة تدريب البيانات.' - }, search: 'بحث', loading: 'جار التحميل...', states: { diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 6fce75d074..d9dc0f9e19 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2567,22 +2567,18 @@ export const en: Translations = { failedRename: 'Failed to rename profile' }, + modelAssignment: { + saveFailed: 'Hermes did not save that model change.', + confirmTitle: 'Model Selection Warning', + confirmDetail: 'Confirm only if you accept this trade-off.', + confirmAction: 'Confirm', + declined: 'Model change cancelled — you declined the data-training tier warning.' + }, + cron: { close: 'Close cron', title: 'Scheduled jobs', count: count => `${count} ${count === 1 ? 'job' : 'jobs'}`, - modelImpact: { - title: 'Scheduled jobs stay on their original model', - message: count => - `${count} unpinned scheduled ${count === 1 ? 'job keeps' : 'jobs keep'} running on the model ${count === 1 ? 'it was' : 'they were'} created under. Pin ${count === 1 ? 'it' : 'them'} or set cron.model to move ${count === 1 ? 'it' : 'them'}.`, - detailMore: (names, remaining) => `${names} and ${remaining} more`, - review: 'Review scheduled jobs', - saveFailed: 'Hermes did not save that model change.', - confirmTitle: 'Model Selection Warning', - confirmDetail: 'Confirm only if you accept this trade-off.', - confirmAction: 'Confirm', - declined: 'Model change cancelled — you declined the data-training tier warning.' - }, 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 6c22b9a13e..fe6f8fff6a 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -2083,22 +2083,18 @@ export const ja = defineLocale({ failedRename: 'プロファイルの名前変更に失敗しました' }, + modelAssignment: { + saveFailed: 'Hermes はモデルの変更を保存しませんでした。', + confirmTitle: 'モデル選択の警告', + confirmDetail: 'このトレードオフを受け入れる場合のみ確認してください。', + confirmAction: '確認', + declined: 'モデル変更をキャンセルしました — データ学習ティアの警告を拒否しました。' + }, + cron: { close: 'Cron を閉じる', title: 'スケジュール済みジョブ', count: count => `${count} 件のジョブ`, - modelImpact: { - title: 'スケジュール済みジョブは元のモデルで実行されます', - message: count => - `ピン留めされていない ${count} 件のスケジュール済みジョブは、作成時のモデルで引き続き実行されます。移行するにはピン留めするか cron.model を設定してください。`, - detailMore: (names, remaining) => `${names}、ほか ${remaining} 件`, - review: 'スケジュール済みジョブを確認', - saveFailed: 'Hermes はモデルの変更を保存しませんでした。', - confirmTitle: 'モデル選択の警告', - confirmDetail: 'このトレードオフを受け入れる場合のみ確認してください。', - confirmAction: '確認', - declined: 'モデル変更をキャンセルしました — データ学習ティアの警告を拒否しました。' - }, search: 'Cron ジョブを検索...', loading: 'Cron ジョブを読み込み中...', states: { diff --git a/apps/desktop/src/i18n/ru.ts b/apps/desktop/src/i18n/ru.ts index 3e05d8cbdb..9c600590ca 100644 --- a/apps/desktop/src/i18n/ru.ts +++ b/apps/desktop/src/i18n/ru.ts @@ -2352,18 +2352,14 @@ export const ru = defineLocale({ failedCreate: 'Не удалось создать профиль', failedRename: 'Не удалось переименовать профиль' }, + modelAssignment: { + saveFailed: 'Hermes не сохранил это изменение модели.' + }, + cron: { close: 'Закрыть cron', title: 'Запланированные задачи', count: count => `${count} ${RU_PLURAL(count, 'задача', 'задачи', 'задач')}`, - modelImpact: { - title: 'Запланированные задачи остаются на исходной модели', - message: count => - `${count} незакреплённых запланированных задач продолжат работать на модели, с которой были созданы. Закрепите их или задайте cron.model, чтобы перевести.`, - 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 16c70e05c8..c9f507570a 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -2181,21 +2181,18 @@ export interface Translations { failedRename: string } + modelAssignment: { + saveFailed: string + confirmTitle: string + confirmDetail: string + confirmAction: string + declined: string + } + cron: { 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 - confirmTitle: string - confirmDetail: string - confirmAction: string - declined: 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 1b44dfceb8..6ea1c64ed8 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -2073,21 +2073,18 @@ export const zhHant = defineLocale({ failedRename: '重新命名設定檔失敗' }, + modelAssignment: { + saveFailed: 'Hermes 未儲存該模型變更。', + confirmTitle: '模型選擇警告', + confirmDetail: '僅在你接受此權衡時確認。', + confirmAction: '確認', + declined: '已取消模型變更 — 你拒絕了資料訓練層級警告。' + }, + cron: { close: '關閉排程', title: '排程工作', count: count => `${count} 個工作`, - modelImpact: { - title: '排程工作將繼續使用原模型', - message: count => `${count} 個未固定的排程工作將繼續使用建立時的模型執行。固定它們或設定 cron.model 以遷移。`, - detailMore: (names, remaining) => `${names}，以及另外 ${remaining} 個`, - review: '檢查排程工作', - saveFailed: 'Hermes 未儲存該模型變更。', - confirmTitle: '模型選擇警告', - confirmDetail: '僅在你接受此權衡時確認。', - confirmAction: '確認', - declined: '已取消模型變更 — 你拒絕了資料訓練層級警告。' - }, search: '搜尋排程工作…', loading: '正在載入排程工作…', states: { diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 14fc8fedaa..f2685a0813 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2720,21 +2720,18 @@ export const zh = defineLocale({ failedRename: '重命名配置档案失败' }, + modelAssignment: { + saveFailed: 'Hermes 未保存该模型更改。', + confirmTitle: '模型选择警告', + confirmDetail: '仅在你接受此权衡时确认。', + confirmAction: '确认', + declined: '已取消模型更改 — 你拒绝了数据训练层级警告。' + }, + cron: { close: '关闭定时任务', title: '定时任务', count: count => `${count} 个任务`, - modelImpact: { - title: '定时任务将继续使用原模型', - message: count => `${count} 个未固定的定时任务将继续使用创建时的模型运行。固定它们或设置 cron.model 以迁移。`, - detailMore: (names, remaining) => `${names}，以及另外 ${remaining} 个`, - review: '检查定时任务', - saveFailed: 'Hermes 未保存该模型更改。', - confirmTitle: '模型选择警告', - confirmDetail: '仅在你接受此权衡时确认。', - confirmAction: '确认', - declined: '已取消模型更改 — 你拒绝了数据训练层级警告。' - }, search: '搜索定时任务…', loading: '正在加载定时任务…', states: { diff --git a/apps/desktop/src/lib/connection-scoped.ts b/apps/desktop/src/lib/connection-scoped.ts index 2ea04e75cb..113a5f050c 100644 --- a/apps/desktop/src/lib/connection-scoped.ts +++ b/apps/desktop/src/lib/connection-scoped.ts @@ -172,8 +172,7 @@ export function connectionScopedAtom( * * Called whenever the window's connection descriptor is published. A null * descriptor is an ordinary disconnect/reconnect state — not evidence the - * user selected another backend — so it keeps the current scope (the same - * contract as syncCronModelImpactConnection). + * user selected another backend — so it keeps the current scope. */ export function rescopeConnectionScopedStores(connection: ConnectionScopeDescriptor | null | undefined): void { if (!connection) { diff --git a/apps/desktop/src/store/cron-model-impact-scope.test.ts b/apps/desktop/src/store/cron-model-impact-scope.test.ts deleted file mode 100644 index fcf7e64fc1..0000000000 --- a/apps/desktop/src/store/cron-model-impact-scope.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -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 deleted file mode 100644 index 24d24a9d15..0000000000 --- a/apps/desktop/src/store/cron-model-impact-scope.ts +++ /dev/null @@ -1,63 +0,0 @@ -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 deleted file mode 100644 index 102f9f0db1..0000000000 --- a/apps/desktop/src/store/cron-model-impact.test.ts +++ /dev/null @@ -1,274 +0,0 @@ -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' - -import { deferred } from '../test/deferred' - -async function waitForConfirmToast() { - return vi.waitFor(() => { - const toast = $notifications.get().find(item => item.id.startsWith('model-warning-confirm-')) - - expect(toast).toBeDefined() - - return toast! - }) -} - -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, - affected_count: 1, - truncated: false, - jobs: [{ id: 'job-1', name, drifted_axes: ['provider', 'model'] }] - } -} - -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('info') - expect(notification?.title).toBe('Scheduled jobs stay on their original model') - expect(notification?.message).toContain('1 unpinned scheduled job keeps running on the model it was created under') - 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, - 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, - affected_count: 0, - truncated: false, - jobs: [] - }) - ) - await setMainModelAssignment({ provider: 'nous', model: 'three' }) - expect($notifications.get()).toEqual([]) - }) - - it('prompts and retries with confirm_expensive_model when the user accepts', async () => { - setModelAssignment.mockResolvedValueOnce(response(positive())) - await setMainModelAssignment({ provider: 'nous', model: 'one' }) - - const confirmResponse = { - ok: false, - scope: 'main', - provider: 'openrouter', - model: 'openai/gpt-5.5-pro', - confirm_required: true, - confirm_message: 'Confirm this expensive model.' - } satisfies ModelAssignmentResponse - - setModelAssignment.mockResolvedValueOnce(confirmResponse) - setModelAssignment.mockResolvedValueOnce(response(positive('Confirmed job'))) - - const pending = setMainModelAssignment({ provider: 'openrouter', model: 'openai/gpt-5.5-pro' }) - const confirm = await waitForConfirmToast() - - expect(confirm.kind).toBe('warning') - expect(confirm.message).toBe('Confirm this expensive model.') - expect(confirm.action?.label).toBe('Confirm') - - confirm.action?.onClick() - await pending - - expect(setModelAssignment).toHaveBeenLastCalledWith( - expect.objectContaining({ - provider: 'openrouter', - model: 'openai/gpt-5.5-pro', - confirm_expensive_model: true - }) - ) - expect($notifications.get().some(item => item.detail?.includes('Confirmed job'))).toBe(true) - }) - - it('declines without retrying when the user dismisses the guard prompt', 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) - - const pending = setMainModelAssignment({ provider: 'openrouter', model: 'openai/gpt-5.5-pro' }) - const confirm = await waitForConfirmToast() - - dismissNotification(confirm.id) - - await expect(pending).rejects.toThrow('Model change cancelled') - expect(setModelAssignment.mock.calls).toHaveLength(2) - - const impact = $notifications.get().find(item => item.id === CRON_MODEL_IMPACT_NOTIFICATION_ID) - const reviewCount = $cronReviewRequest.get() - impact?.action?.onClick() - expect($cronReviewRequest.get()).toBe(reviewCount + 1) - }) - - it('fails closed without a prompt when skipConfirmPrompt is set', async () => { - 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' }, undefined, { - skipConfirmPrompt: true - }) - ).rejects.toThrow('Confirm this expensive model.') - expect(setModelAssignment).toHaveBeenCalledTimes(1) - expect($notifications.get()).toEqual([]) - }) - - it('does not recurse when the backend still demands confirm after ack', async () => { - const confirmResponse = { - ok: false, - scope: 'main', - provider: 'openrouter', - model: 'openai/gpt-5.5-pro', - confirm_required: true, - confirm_message: 'Confirm this expensive model.' - } satisfies ModelAssignmentResponse - - setModelAssignment.mockResolvedValueOnce(confirmResponse) - setModelAssignment.mockResolvedValueOnce(confirmResponse) - - const pending = setMainModelAssignment({ provider: 'openrouter', model: 'openai/gpt-5.5-pro' }) - const confirm = await waitForConfirmToast() - - confirm.action?.onClick() - - await expect(pending).rejects.toThrow('Confirm this expensive model.') - expect(setModelAssignment).toHaveBeenCalledTimes(2) - }) - - 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('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 deleted file mode 100644 index 8dc38435b0..0000000000 --- a/apps/desktop/src/store/cron-model-impact.ts +++ /dev/null @@ -1,263 +0,0 @@ -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' || - !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.affected_count === 0) { - dismissNotification(CRON_MODEL_IMPACT_NOTIFICATION_ID) - - return - } - - // Informational: these jobs keep running on the model they were created under; nothing is - // skipped. The action is a read-only review so the user can pin or move them deliberately. - notify({ - id: CRON_MODEL_IMPACT_NOTIFICATION_ID, - kind: 'info', - 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, - scopeProfile?: null | string, - options?: { skipConfirmPrompt?: boolean } -): Promise { - const { connection, generation } = beginCronModelImpactAssignment() - const profile = profileIdentity() - - // Only pass the extra arg when a scope override exists, so unscoped callers - // keep the exact legacy call shape. - const assign = (body: Omit) => - scopeProfile == null - ? setModelAssignment({ ...body, scope: 'main' }) - : setModelAssignment({ ...body, scope: 'main' }, scopeProfile) - - let result = await assign(request) - - // Backend demands an explicit ack before persisting a model that trips a - // selection guard (expensive / data-training tiers like *-contributor). - // Settings used to throw confirm_message as a red error, so Apply could - // never persist. Prompt, then retry with confirm_expensive_model. - if (result.confirm_required) { - if (request.confirm_expensive_model || options?.skipConfirmPrompt) { - // Already acked, or headless onboarding (nothing mounted to click). - // Fail closed instead of recursing / dangling a prompt. - throw new Error(result.confirm_message?.trim() || translateNow('cron.modelImpact.saveFailed')) - } - - const accepted = await confirmModelWarning(result.confirm_message?.trim() ?? '') - - if (!accepted) { - throw new Error(translateNow('cron.modelImpact.declined')) - } - - result = await assign({ ...request, confirm_expensive_model: true }) - - if (result.confirm_required || result.ok !== true) { - throw new Error(result.confirm_message?.trim() || translateNow('cron.modelImpact.saveFailed')) - } - } else if (result.ok !== true) { - throw new Error(result.confirm_message?.trim() || translateNow('cron.modelImpact.saveFailed')) - } - - // A scoped assignment targets ANOTHER profile's backend: its cron impact - // belongs to that profile, and the review action would open the ACTIVE - // profile's cron view — skip the warning rather than mis-route it. - if (scopeProfile != null) { - return result - } - - 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)) - -/** - * Selection-guard warning as a confirm toast. Resolves true on Confirm, false - * on dismiss. The desktop has no blocking confirm API; this is the same - * notify-with-action pattern the in-session model picker uses. - */ -function confirmModelWarning(message: string): Promise { - const id = `model-warning-confirm-${Date.now()}` - - return new Promise(resolve => { - let settled = false - - const finish = (value: boolean) => { - if (settled) { - return - } - - settled = true - dismissNotification(id) - resolve(value) - } - - notify({ - id, - kind: 'warning', - title: translateNow('cron.modelImpact.confirmTitle'), - message: message || translateNow('cron.modelImpact.confirmDetail'), - detail: translateNow('cron.modelImpact.confirmDetail'), - action: { - label: translateNow('cron.modelImpact.confirmAction'), - onClick: () => finish(true) - }, - onDismiss: () => finish(false) - }) - }) -} diff --git a/apps/desktop/src/store/model-assignment.ts b/apps/desktop/src/store/model-assignment.ts new file mode 100644 index 0000000000..c044e1f7a9 --- /dev/null +++ b/apps/desktop/src/store/model-assignment.ts @@ -0,0 +1,83 @@ +import { setModelAssignment } from '@/hermes' +import { translateNow } from '@/i18n' +import { dismissNotification, notify } from '@/store/notifications' +import type { ModelAssignmentRequest, ModelAssignmentResponse } from '@/types/hermes' + +/** + * Selection-guard warning as a confirm toast. Resolves true on Confirm, false + * on dismiss. The desktop has no blocking confirm API; this is the same + * notify-with-action pattern the in-session model picker uses. + */ +function confirmModelWarning(message: string): Promise { + const id = `model-warning-confirm-${Date.now()}` + + return new Promise(resolve => { + let settled = false + + const finish = (value: boolean) => { + if (settled) { + return + } + + settled = true + dismissNotification(id) + resolve(value) + } + + notify({ + id, + kind: 'warning', + title: translateNow('modelAssignment.confirmTitle'), + message: message || translateNow('modelAssignment.confirmDetail'), + detail: translateNow('modelAssignment.confirmDetail'), + action: { + label: translateNow('modelAssignment.confirmAction'), + onClick: () => finish(true) + }, + onDismiss: () => finish(false) + }) + }) +} + +export async function setMainModelAssignment( + request: Omit, + scopeProfile?: null | string, + options?: { skipConfirmPrompt?: boolean } +): Promise { + // Only pass the extra arg when a scope override exists, so unscoped callers + // keep the exact legacy call shape. + const assign = (body: Omit) => + scopeProfile == null + ? setModelAssignment({ ...body, scope: 'main' }) + : setModelAssignment({ ...body, scope: 'main' }, scopeProfile) + + let result = await assign(request) + + // Backend demands an explicit ack before persisting a model that trips a + // selection guard (expensive / data-training tiers like *-contributor). + // Settings used to throw confirm_message as a red error, so Apply could + // never persist. Prompt, then retry with confirm_expensive_model. + if (result.confirm_required) { + if (request.confirm_expensive_model || options?.skipConfirmPrompt) { + // Already acked, or headless onboarding (nothing mounted to click). + // Fail closed instead of recursing / dangling a prompt. + throw new Error(result.confirm_message?.trim() || translateNow('modelAssignment.saveFailed')) + } + + const accepted = await confirmModelWarning(result.confirm_message?.trim() ?? '') + + if (!accepted) { + throw new Error(translateNow('modelAssignment.declined')) + } + + result = await assign({ ...request, confirm_expensive_model: true }) + + if (result.confirm_required || result.ok !== true) { + throw new Error(result.confirm_message?.trim() || translateNow('modelAssignment.saveFailed')) + } + } else if (result.ok !== true) { + throw new Error(result.confirm_message?.trim() || translateNow('modelAssignment.saveFailed')) + } + + return result +} diff --git a/apps/desktop/src/store/onboarding.ts b/apps/desktop/src/store/onboarding.ts index b81b650bca..e839e9fc61 100644 --- a/apps/desktop/src/store/onboarding.ts +++ b/apps/desktop/src/store/onboarding.ts @@ -15,8 +15,8 @@ import { import { translateNow } from '@/i18n' import { isProviderSetupErrorMessage } from '@/lib/provider-setup-errors' import { evaluateRuntimeReadiness, type RuntimeReadinessResult } from '@/lib/runtime-readiness' -import { setMainModelAssignment } from '@/store/cron-model-impact' import { ackFreeTierNotice, freeTierReadyPending, refreshFreeTierStatus, setFreeTierRoute } from '@/store/free-tier' +import { setMainModelAssignment } from '@/store/model-assignment' import { notify, notifyError } from '@/store/notifications' import { guidedOnboardingActive } from '@/store/onboarding-gate' import type { OAuthProvider, OAuthStartResponse } from '@/types/hermes' diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index b118c9c000..a32e5aad08 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -16,7 +16,6 @@ import { } from '@/lib/storage' import { withTimeout } from '@/lib/with-timeout' import { $connectionsRegistry } from '@/store/connection-registry-state' -import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' import { $gateway, activeGatewayConnectionId, @@ -440,7 +439,6 @@ $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 63009562e8..af544e8e83 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -12,7 +12,6 @@ import { rescopeConnectionScopedStores } from '@/lib/connection-scoped' import { persistBoolean, persistString, readJson, storedBoolean, storedString, writeJson } from '@/lib/storage' -import { syncCronModelImpactConnection } from '@/store/cron-model-impact-scope' import type { SessionInfo, UsageStats } from '@/types/hermes' import { isSessionRemovalPending } from './session-removal' @@ -1266,7 +1265,6 @@ export const setConnection = (next: Updater) => { // consumer reconciles against it. A null descriptor (reconnect blip) // keeps the current scope. rescopeConnectionScopedStores($connection.get()) - syncCronModelImpactConnection($connection.get()) // Null descriptor = reconnect blip; keep the last resolved mode (same // contract as rescopeConnectionScopedStores above). diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 0aaead6015..2330d0405c 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -1543,21 +1543,6 @@ 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 - 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 { @@ -1726,8 +1711,6 @@ 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 diff --git a/cli.py b/cli.py index 1e4d5a5d9b..7db145d0a4 100644 --- a/cli.py +++ b/cli.py @@ -2412,10 +2412,6 @@ def save_config_value(key_path: str, value: any) -> bool: os.chmod(config_path, 0o600) except (OSError, NotImplementedError): pass - # Same unpinned-cron notice as `hermes config set` for every model switch. - from hermes_cli.config import warn_unpinned_cron_jobs_after_model_config_change - - warn_unpinned_cron_jobs_after_model_config_change(key_path, value) return True except Exception as e: logger.error("Failed to save config: %s", e) diff --git a/compat_manifest.json b/compat_manifest.json index b214cfbc85..292ee175ee 100644 --- a/compat_manifest.json +++ b/compat_manifest.json @@ -5468,12 +5468,6 @@ "kind": "import", "target": "binascii" }, - { - "facade": "hermes_cli.web_server", - "name": "build_cron_model_impact", - "kind": "moved-lazy", - "target": "hermes_cli.config" - }, { "facade": "hermes_cli.web_server", "name": "bulk_delete_sessions_endpoint", @@ -6686,12 +6680,6 @@ "kind": "moved-lazy", "target": "hermes_cli.web_routers.ops" }, - { - "facade": "hermes_cli.web_server", - "name": "resolve_cron_model_drift_defaults", - "kind": "moved-lazy", - "target": "hermes_cli.config" - }, { "facade": "hermes_cli.web_server", "name": "resolve_gateway_liveness", diff --git a/cron/jobs.py b/cron/jobs.py index fceda2f33e..34eb29581b 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -1561,27 +1561,24 @@ def _normalize_workdir(workdir: Optional[str]) -> Optional[str]: return str(resolved) -def _resolve_default_model_snapshot() -> Optional[str]: - """Default model resolved as the ticker's ``run_job`` does, so unpinned jobs can snapshot it and - keep running on it after a later swap. ``None`` on missing config or failure ("no snapshot").""" - try: - from hermes_cli.config_effective import load_user_config_effective +def _main_model_pin() -> Tuple[Optional[str], Optional[str]]: + """``(provider, model)`` the main agent runs on right now (``model.default`` + the provider it + resolves to), for ``pinned=True`` jobs: the lock is a plain per-job pin, so the scheduler needs + no second precedence axis. ``(None, None)`` when nothing is configured (the job stays unpinned).""" + from hermes_cli.config_effective import load_user_config_effective - cfg_path = get_hermes_home() / "config.yaml" - if not cfg_path.exists(): - return None - cfg = load_user_config_effective(cfg_path) - cron_cfg = cfg.get("cron") or {} - if isinstance(cron_cfg, dict): - cron_model = cron_cfg.get("model") - if isinstance(cron_model, str) and cron_model.strip(): - return cron_model.strip() - model_cfg = cfg.get("model") or {} - if isinstance(model_cfg, dict): - model_cfg = model_cfg.get("default") or model_cfg.get("model") - return model_cfg.strip() or None if isinstance(model_cfg, str) else None - except Exception: - return None + cfg_path = get_hermes_home() / "config.yaml" + cfg = load_user_config_effective(cfg_path) if cfg_path.exists() else {} + model_cfg = cfg.get("model") or {} + model = model_cfg.get("default") or model_cfg.get("model") if isinstance(model_cfg, dict) else model_cfg + model = _normalize_job_optional_text(model) + if not model: + return None, None + provider = None + with contextlib.suppress(Exception): + from hermes_cli.runtime_provider import resolve_runtime_provider + provider = _normalize_job_optional_text(resolve_runtime_provider(requested=None).get("provider")) + return (provider.lower() if provider else None), model def _normalize_job_optional_text( @@ -1661,50 +1658,6 @@ _UPDATE_FIELD_NORMALIZERS: Dict[str, Callable[[Any], Any]] = { } -def _compute_provider_model_snapshots( - *, provider: Any, model: Any, base_url: Any, no_agent: Any, -) -> Tuple[Optional[str], Optional[str]]: - """Snapshot unpinned provider/model resolution: the scheduler runs the job on this snapshot after - a later global switch instead of silently changing spend. Pinned axes and no-agent jobs carry no - snapshot.""" - normalized_provider = _normalize_job_optional_text(provider) - normalized_model = _normalize_job_optional_text(model) - normalized_base_url = _normalize_base_url(base_url) - if bool(no_agent): - return None, None - - provider_snapshot: Optional[str] = None - model_snapshot: Optional[str] = None - if normalized_provider is None: - with contextlib.suppress(Exception): - from hermes_cli.runtime_provider import resolve_runtime_provider - - runtime_kwargs = {"requested": None} - # Delegate all rate-limit / 5xx retry to hermes's outer conversation loop, which honors - # Retry-After. The SDK default (max_retries=2) uses its own 1-2s backoff that ignores - # Retry-After and double-retries inside our loop — burning request slots against a bucket that - # won't refill for minutes. (#26293) - if normalized_base_url: - runtime_kwargs["explicit_base_url"] = normalized_base_url - snap = resolve_runtime_provider(**runtime_kwargs) - provider_snapshot = str(snap.get("provider") or "").strip().lower() or None - if normalized_model is None: - with contextlib.suppress(Exception): - model_snapshot = _resolve_default_model_snapshot() or None - return provider_snapshot, model_snapshot - - -def _normalized_inference_axes( - job: Dict[str, Any], -) -> Tuple[Optional[str], Optional[str], Optional[str], bool]: - """Return the stored inference-routing fields in their semantic form.""" - return ( - _normalize_job_optional_text(job.get("provider")), - _normalize_job_optional_text(job.get("model")), _normalize_base_url(job.get("base_url")), - bool(job.get("no_agent")), - ) - - def _validate_job_mode_invariants( monitor_script: Optional[str], monitor_url: Optional[str], @@ -1771,6 +1724,7 @@ def create_job( failure_deliver: Optional[str] = None, paused: bool = False, paused_reason: Optional[str] = None, + pinned: bool = False, ) -> Dict[str, Any]: """Create a new cron job and return the stored record. @@ -1819,8 +1773,8 @@ def create_job( or "cron job" ) name = name or label_source[:50].strip() - provider_snapshot, model_snapshot = _compute_provider_model_snapshots( - provider=f["provider"], model=f["model"], base_url=f["base_url"], no_agent=f["no_agent"]) + if pinned and not f["model"]: + f["provider"], f["model"] = _main_model_pin() next_run_at = _next_run_or_reject_past_oneshot(parsed_schedule, name, schedule, "") job = { @@ -1831,8 +1785,6 @@ def create_job( "skill": normalized_skills[0] if normalized_skills else None, "model": f["model"], "provider": f["provider"], - "provider_snapshot": provider_snapshot, - "model_snapshot": model_snapshot, "base_url": f["base_url"], "script": f["script"], "no_agent": f["no_agent"], @@ -1944,6 +1896,22 @@ def _reject_terminal_activation(job: Dict[str, Any], updated: Dict[str, Any], jo "through update_job; use cron resume --run-now or --at.") +def _apply_pin_update(job: Dict[str, Any], updates: Dict[str, Any]) -> None: + """``pinned`` is not stored; it rewrites the per-job pin. ``True`` with no explicit model locks + the main agent's current provider+model onto the job, ``False`` releases both so the job follows + the main model again (an explicit ``model`` in the same update wins over either).""" + if "pinned" not in updates: + return + pinned = bool(updates.pop("pinned")) + if "model" in updates: + return + if pinned: + if not _normalize_job_optional_text(job.get("model")): + updates["provider"], updates["model"] = _main_model_pin() + else: + updates["provider"], updates["model"] = None, None + + def _normalize_job_updates(job: Dict[str, Any], updates: Dict[str, Any]) -> None: """Normalize updates in place like create_job; invalid values raise BEFORE the merge. ``repeat`` accepts the stored dict or a bare value (coerced, completed counter preserved).""" @@ -2035,7 +2003,7 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] def apply(jobs, i, job): _rederive_repeat_for_schedule_change(job, updates) _normalize_job_updates(job, updates) - previous_inference_axes = _normalized_inference_axes(job) + _apply_pin_update(job, updates) updated = _apply_skill_fields({**job, **updates}) _reject_terminal_activation(job, updated, job_id) # Re-check on the MERGED record; scoped to changed fields so legacy records keep loading. @@ -2047,10 +2015,6 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] _normalize_job_optional_text(updated.get("script"))) if any(k in updates for k in _PAYLOAD_FIELDS) and job_payload_is_empty(updated): raise ValueError(EMPTY_PAYLOAD_ERROR) - inference_fields_changed = bool( - {"provider", "model", "base_url", "no_agent"}.intersection(updates) - ) and _normalized_inference_axes(updated) != previous_inference_axes - if "schedule" in updates: _apply_schedule_update(updated, updates, job_id) # next_run_at now follows the new schedule; a stale quota_hold_until would only shield @@ -2062,13 +2026,6 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] # An explicit schedule/lifecycle rewrite supersedes any occurrence the dispatcher # left unclaimed — pause/resume/edit must not resurrect a slot from before the edit. updated.pop("pending_slot", None) - if inference_fields_changed: - snapshots = _compute_provider_model_snapshots( - provider=updated.get("provider"), - model=updated.get("model"), - base_url=updated.get("base_url"), - no_agent=updated.get("no_agent")) - updated["provider_snapshot"], updated["model_snapshot"] = snapshots _fill_missing_next_run(updated) _reject_terminal_activation(job, updated, job_id) jobs[i] = updated @@ -2078,78 +2035,6 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] return _with_job(job_id, apply) -def resnapshot_job(job_id: str) -> Optional[Dict[str, Any]]: - """Refresh provider/model snapshots for a job's UNPINNED axes to the - current global resolution. - - This is the "adopt the current global default" companion to pinning - (#44585). Where pinning a job (``provider=... model=...``) makes it stop - tracking the global default forever, ``resnapshot_job`` re-captures the - current resolution so an unpinned job follows the user's deliberately - changed default — while remaining unpinned and tracking future changes. - - Semantics: - - Pinned axes (job has an explicit provider/model) keep their snapshot - None and are left untouched. - - no_agent script jobs carry no snapshot and are left untouched. - - If the current resolution fails, the previous snapshot is left in - place (fail-open, matching create_job semantics). - - Makes no inference call — it only recomputes the snapshot string from - config. Returns the normalized updated job, or None if not found. - """ - job = resolve_job_ref(job_id) - if not job: - return None - provider_snapshot, model_snapshot = _compute_provider_model_snapshots( - provider=job.get("provider"), - model=job.get("model"), - base_url=job.get("base_url"), - no_agent=job.get("no_agent"), - ) - jobs = load_jobs() - for i, stored in enumerate(jobs): - if stored["id"] != job["id"]: - continue - jobs[i]["provider_snapshot"] = provider_snapshot - jobs[i]["model_snapshot"] = model_snapshot - save_jobs(jobs) - return _normalize_job_record(jobs[i]) - return None - - -def resnapshot_all_unpinned() -> List[Dict[str, Any]]: - """Refresh provider/model snapshots for every job that has any unpinned - axis, adopting the current global resolution for each. - - Skips no_agent jobs and jobs pinned on all inference axes (nothing - unpinned to refresh). Equivalent to calling ``resnapshot_job`` for each - eligible job. Returns the list of updated jobs. - """ - updated: List[Dict[str, Any]] = [] - jobs = load_jobs() - changed = False - for job in jobs: - if bool(job.get("no_agent")): - continue - if job.get("provider") and job.get("model"): - # Pinned on every axis — nothing unpinned to refresh. - continue - provider_snapshot, model_snapshot = _compute_provider_model_snapshots( - provider=job.get("provider"), - model=job.get("model"), - base_url=job.get("base_url"), - no_agent=job.get("no_agent"), - ) - job["provider_snapshot"] = provider_snapshot - job["model_snapshot"] = model_snapshot - changed = True - updated.append(_normalize_job_record(job)) - if changed: - save_jobs(jobs) - return updated - - def pause_job(job_id: str, reason: Optional[str] = None) -> Optional[Dict[str, Any]]: """Pause a job without deleting it. Accepts a job ID or name.""" job = resolve_job_ref(job_id) diff --git a/cron/scheduler.py b/cron/scheduler.py index 47f4c3e70a..b015f539ab 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -39,7 +39,7 @@ from hermes_constants import get_hermes_home, hermes_home_key from cron.env_settings import cron_env_setting from hermes_cli._subprocess_compat import windows_hide_flags from hermes_cli.config import ( - load_config, load_config_readonly, resolve_cron_model_drift_defaults) + load_config, load_config_readonly) from hermes_cli.fallback_config import get_fallback_chain from hermes_time import now as _hermes_now from agent.interrupt_compat import request_hard_interrupt @@ -1377,27 +1377,10 @@ class _CronJobConfig: cron_default_provider: str -def _snapshot_pin(job: dict, axis: str, current: str, job_id: str) -> str: - """The creation snapshot is an unpinned axis's effective pin: return it, logging once when it - differs from *current* (the live global default); ``""`` for legacy jobs without one, which keep - following the global default. A global model/provider change must never stop a cron job; a job - keeps running on what it was created under until the operator pins it or sets a cron.* fleet - default (#44585).""" - snapshot = str(job.get(f"{axis}_snapshot") or "").strip() - if snapshot and current and snapshot.lower() != current.lower(): - logger.info( - "Job '%s': running on creation-snapshot %s %r (global default is now %r); " - "`hermes cron resnap %s` adopts the new default (stays unpinned), " - "`hermes cron edit %s --%s ` or cron.%s in config.yaml pins it.", - job_id, axis, snapshot, current, job_id, job_id, axis, - "model" if axis == "model" else "model_provider") - return snapshot - - def _load_cron_job_config(job: dict, job_id: str, job_name: str) -> _CronJobConfig: - """Load config.yaml and resolve the run's model: per-job override > cron.model (fleet default) > - creation snapshot > HERMES_MODEL > config ``model:``. Re-read every tick (no cache) so - ``hermes cron edit --model`` applies next tick.""" + """Load config.yaml and resolve the run's model: per-job pin > cron.model (fleet default) > + the main agent model (config ``model:``, then HERMES_MODEL). Re-read every tick (no cache) so + ``hermes cron edit --model`` and ``hermes model`` both apply next tick.""" model = job.get("model") or cron_env_setting("HERMES_MODEL") or "" _cron_default_provider = "" _cfg: dict = {} @@ -1418,9 +1401,11 @@ def _load_cron_job_config(job: dict, job_id: str, job_name: str) -> _CronJobConf if _cron_default_model: model = _cron_default_model else: - _, _global_model = resolve_cron_model_drift_defaults( - _cfg, environ={"HERMES_MODEL": cron_env_setting("HERMES_MODEL")}) - model = _snapshot_pin(job, "model", _global_model, job_id) or _global_model or model + # The main agent model: ``model: `` shorthand or ``model.default``. + _main = _model_cfg if isinstance(_model_cfg, str) else ( + _model_cfg.get("default") or _model_cfg.get("model") or _model_cfg.get("name") + if isinstance(_model_cfg, dict) else "") + model = str(_main or "").strip() or model except Exception as e: logger.warning("Job '%s': failed to load config.yaml, using defaults: %s", job_id, e) @@ -1530,20 +1515,14 @@ def _blocked_config_result(job_id: str, job_name: str, _pf_reason: str) -> tuple def _resolve_job_runtime(job: dict, job_id: str, jc: _CronJobConfig) -> tuple[dict, str]: """Resolve the runtime, walking the fallback chain on auth/transient-network errors. Returns ``(runtime, model)``; provider+model swap atomically (never swap only the provider while keeping - a paid primary model). Provider precedence: per-job pin > cron.model_provider > creation - snapshot > persisted global config.""" + a paid primary model). Provider precedence: per-job pin > cron.model_provider > persisted + global config (None lets resolve_runtime_provider read it).""" from hermes_cli.runtime_provider import ( resolve_runtime_provider, format_runtime_provider_error) from hermes_cli.auth import AuthError model = jc.model requested = job.get("provider") or jc.cron_default_provider or None - if not requested: - global_provider = ( - str(jc.model_cfg.get("provider") or "").strip() if isinstance(jc.model_cfg, dict) else "") - # None (not the config provider) keeps the legacy no-snapshot path resolving from persisted - # config exactly as before. - requested = _snapshot_pin(job, "provider", global_provider, job_id) or None try: # Do NOT pass HERMES_INFERENCE_PROVIDER as `requested`: it would override persisted config # and resurrect stale providers for unpinned jobs. diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 476c517ab8..3cc1a1fc14 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3101,172 +3101,6 @@ def edit_config(): subprocess.run([editor, str(config_path)]) -# ---- Cron model-drift helpers: which unpinned jobs stay on their creation snapshot ---- - -_CRON_DRIFT_AXIS_BY_KEY = { - "model": "model", "model.default": "model", "model.model": "model", "model.name": "model", - "model.provider": "provider", "provider": "provider"} - - -def _cron_model_drift_axis_for_config_key(key: str) -> Optional[str]: - """Return the cron inference axis affected by a config key, if any.""" - return _CRON_DRIFT_AXIS_BY_KEY.get(str(key or "").strip().lower()) - - -def _cron_section(config: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]: - """Return the ``cron`` mapping of *config* (loading the merged config when None), else None.""" - if config is None: - try: - config = load_config() - except Exception: - return None - cron_config = config.get("cron") if isinstance(config, dict) else None - return cron_config if isinstance(cron_config, dict) else None - - -_CRON_MODEL_IMPACT_JOB_LIMIT = 50 -_CRON_MODEL_IMPACT_ID_LIMIT = 256 -_CRON_MODEL_IMPACT_NAME_LIMIT = 120 - - -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)`` cron compares against snapshots. - Mirrors the scheduler's precedence: a truthy configured model wins over ``HERMES_MODEL``; the - environment is only a fallback. Per-job and cron fleet defaults are handled by the caller - because they cover an axis rather than changing the global assignment.""" - env = os.environ if environ is None else environ - provider = "" - model_config = config.get("model") if isinstance(config, dict) else None - if isinstance(model_config, dict): - provider = _model_assignment_text(model_config.get("provider")) - model_config = model_config.get("default") or model_config.get("model") or model_config.get("name") - configured_model = _model_assignment_text(model_config) - return provider, configured_model or _model_assignment_text(env.get("HERMES_MODEL", "")) - - -def cron_model_drift_axes( - job: Any, *, current_provider: Any = "", current_model: Any = "", config: Any = None -) -> List[str]: - """Return the unpinned axes on which *job* will keep running on its creation snapshot rather - than the new global assignment (the scheduler treats the snapshot as the effective pin).""" - if not isinstance(job, dict): - return [] - - current = { - "provider": _model_assignment_text(current_provider).lower(), - "model": _model_assignment_text(current_model).lower()} - # A cron.model / cron.model_provider fleet default covers its axis: that axis never reads the - # snapshot at fire time, so reporting it would be false. - fleet = _cron_section(config) or {} - drifted: List[str] = [] - for axis, fleet_key in (("provider", "model_provider"), ("model", "model")): - if _model_assignment_text(fleet.get(fleet_key)) or _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 _is_control_char(char: str) -> bool: - return unicodedata.category(char).startswith("C") - - -def _valid_cron_impact_job_id(value: Any) -> str: - job_id = value.strip() if isinstance(value, str) else "" - if len(job_id) > _CRON_MODEL_IMPACT_ID_LIMIT or any(map(_is_control_char, 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 _is_control_char(char)) - 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 _cron_model_impact_result(available: bool) -> Dict[str, Any]: - return {"available": available, "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 unpinned jobs that stay on their creation snapshot - after a global model/provider change. Job-store inspection is best effort: the model assignment - has already succeeded when Desktop requests this, so an unreadable store is reported as - unavailable rather than failing.""" - if jobs is None: - try: - from cron.jobs import load_jobs - - jobs = load_jobs() - except Exception: - return _cron_model_impact_result(False) - if not isinstance(jobs, list): - return _cron_model_impact_result(False) - - result = _cron_model_impact_result(True) - - from cron.jobs import is_job_runnable - - seen_ids: Set[str] = set() - for job in jobs: - if not isinstance(job, dict) or 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 warn_unpinned_cron_jobs_after_model_config_change( - key: str, value: Any, config: Optional[Dict[str, Any]] = None) -> None: - """Tell the operator which unpinned cron jobs a global model/provider change does NOT move.""" - axis = _cron_model_drift_axis_for_config_key(key) - if axis is None: - return - - new_value = _model_assignment_text(value) - if not new_value: - return - impact = build_cron_model_impact( - current_provider=new_value if axis == "provider" else "", - current_model=new_value if axis == "model" else "", config=config, jobs=None) - affected = impact["affected_count"] - if affected <= 0: - return - - noun, verb = ("job", "keeps") if affected == 1 else ("jobs", "keep") - print( - f"ℹ️ {affected} unpinned cron {noun} {verb} running on the {axis} it was created under " - f"(its {axis}_snapshot), not the new global {axis}. To move it, pin it with " - "`hermes cron edit --provider --model ` or set a fleet default " - "with `hermes config set cron.model `.") - - def _default_value_for_key(dotted_key: str): """Return the leaf value declared for *dotted_key* in ``DEFAULT_CONFIG`` (None for dicts/misses).""" node = cfg_get(DEFAULT_CONFIG, *_split_key_path(dotted_key)) @@ -3774,7 +3608,6 @@ def set_config_value(key: str, value: str, force: bool = False): print(f"✓ Set {key} = {_display_value} in {config_path}") if _route_notice: print(_route_notice) - warn_unpinned_cron_jobs_after_model_config_change(key, value, user_config) # Post-write unknown-key notice (#34067): value IS saved, but tell the user the runtime may never read # it and suggest the likely-intended path. diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 5bcb40046d..6bfcd5d5fb 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -1750,9 +1750,8 @@ DEFAULT_CONFIG = { # False = fail during the run instead. "preflight": True, # Default model for cron jobs (WHAT model runs). Fire-time resolution: per-job pin > - # cron.model > the job's creation-time snapshot > model.default. An unpinned job keeps - # running on the model it was created under when model.default later changes; cron.model - # is the way to move the whole fleet at once. "" = fall through. + # cron.model > model.default (the main agent model). An unpinned job follows the main + # model on every run; cron.model decouples the whole fleet from chat. "" = fall through. "model": "", # Inference provider paired with cron.model (NOT the scheduler provider below). "" = resolve # from global config. diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index fa459339d2..af415ab86e 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -644,7 +644,7 @@ def cron_doctor() -> int: _JOB_ARG_FIELDS = (("name", "name"), ("deliver", "deliver"), ("failure_deliver", "failure_deliver"), ("repeat", "repeat"), ("script", "script"), ("workdir", "workdir"), - ("model", "model"), ("provider", "model_provider"), + ("model", "model"), ("provider", "model_provider"), ("pinned", "pinned"), ("monitor_script", "monitor_script"), ("monitor_url", "monitor_url"), ("continuity", "continuity"), ("reasoning_effort", "reasoning_effort")) @@ -868,7 +868,7 @@ _CRON_SUBCOMMANDS = { "resume": lambda a: cron_resume(a), "run": lambda a: _job_action("run", a.job_id, "Triggered"), "remove": lambda a: _job_action("remove", a.job_id, "Removed"), - "resnap": lambda a: _cron_resnap(a)} +} _CRON_SUBCOMMANDS["history"] = _CRON_SUBCOMMANDS["runs"] _CRON_SUBCOMMANDS["add"] = _CRON_SUBCOMMANDS["create"] _CRON_SUBCOMMANDS["rm"] = _CRON_SUBCOMMANDS["delete"] = _CRON_SUBCOMMANDS["remove"] @@ -881,35 +881,5 @@ def cron_command(args): if handler is not None: return handler(args) print(f"Unknown cron command: {subcmd}\n" - "Usage: hermes cron [list|create|edit|pause|resume|run|remove|resnap|status|runs|doctor|tick]") + "Usage: hermes cron [list|create|edit|pause|resume|run|remove|status|runs|doctor|tick]") sys.exit(1) - - -def _cron_resnap(args) -> int: - """Handle `hermes cron resnap [job_id] [--all]`.""" - if bool(getattr(args, "all", False)): - result = _cron_api(action="resnap", all=True) - if not result.get("success"): - print(color(f"Failed to resnap: {result.get('error', 'unknown error')}", Colors.RED)) - return 1 - updated = result.get("updated_jobs", []) - print(color(f"Resnapped {len(updated)} unpinned job(s) to the current global resolution.", Colors.GREEN)) - for job in updated: - print(f" • {job.get('name', job.get('job_id'))} ({job.get('job_id')})") - if not updated: - print(" (no unpinned agent jobs found — nothing to refresh)") - return 0 - - job_id = getattr(args, "job_id", None) - if not job_id: - print(color("resnap requires either a or --all.", Colors.RED)) - print("Usage: hermes cron resnap | hermes cron resnap --all") - return 1 - result = _cron_api(action="resnap", job_id=job_id) - if not result.get("success"): - print(color(f"Failed to resnap job: {result.get('error', 'unknown error')}", Colors.RED)) - return 1 - job = result.get("job", {}) - print(color(f"Resnapped job: {job.get('name', job_id)} ({job.get('job_id', job_id)})", Colors.GREEN)) - print(" Adopted the current global inference resolution; the job remains unpinned and will track future global changes.") - return 0 diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index b8057addaa..0124fef71e 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1755,13 +1755,11 @@ def persist_model_selection(result: ModelSwitchResult, config_path: Any = None) user set there (``model_slots``, ``model_fallback``, ...). ``should_clear_context_pin`` can do cold-start disk I/O — async callers run this on a worker thread.""" from pathlib import Path - from hermes_cli.config import get_config_path, read_user_config_raw, warn_unpinned_cron_jobs_after_model_config_change + from hermes_cli.config import get_config_path, read_user_config_raw from utils import atomic_roundtrip_yaml_update path = Path(config_path) if config_path else get_config_path() for key, value in model_selection_config_updates(result, read_user_config_raw(path).get("model")).items(): atomic_roundtrip_yaml_update(path, f"model.{key}", value) - # Same unpinned-cron notice as `hermes config set` for every model switch. - warn_unpinned_cron_jobs_after_model_config_change(f"model.{key}", value) try: # owner-only: config files contain API keys os.chmod(path, 0o600) except (OSError, NotImplementedError): diff --git a/hermes_cli/subcommands/cron.py b/hermes_cli/subcommands/cron.py index f037dfd171..9baa530c86 100644 --- a/hermes_cli/subcommands/cron.py +++ b/hermes_cli/subcommands/cron.py @@ -66,7 +66,10 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None: cron_create.add_argument("--model", help="Pin this job to a specific inference model (user-owned; the " "agent's cronjob tool cannot set this). Omit to follow " - "cron.model / model.default from config.yaml.") + "cron.model, then the main agent model (`hermes model`), at fire time.") + cron_create.add_argument("--pin", dest="pinned", action="store_true", default=None, + help="Lock the CURRENT main agent model (and its provider) onto this job so later " + "`hermes model` changes never touch it. Ignored when --model is given.") cron_create.add_argument("--provider", dest="model_provider", help="Inference provider paired with --model (e.g. 'openrouter', 'nous').") cron_create.add_argument("--reasoning-effort", dest="reasoning_effort", @@ -129,7 +132,12 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None: cron_edit.add_argument("--model", help="Pin this job to a specific inference model (user-owned; the " "agent's cronjob tool cannot set this). Pass empty string to " - "clear the pin and follow cron.model / model.default.") + "clear the pin and follow cron.model, then the main agent model.") + _pin = cron_edit.add_mutually_exclusive_group() + _pin.add_argument("--pin", dest="pinned", action="store_true", default=None, + help="Lock the CURRENT main agent model (and its provider) onto this job.") + _pin.add_argument("--unpin", dest="pinned", action="store_false", + help="Release the job's model pin so it follows the main agent model again.") cron_edit.add_argument("--provider", dest="model_provider", help="Inference provider paired with --model. Pass empty string to clear.") cron_edit.add_argument("--reasoning-effort", dest="reasoning_effort", @@ -154,23 +162,6 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None: "remove", aliases=["rm", "delete"], help="Remove a scheduled job") cron_remove.add_argument("job_id", help="Job ID to remove") - cron_resnap = cron_subparsers.add_parser( - "resnap", - help=( - "Adopt the current global inference resolution for unpinned jobs " - "without pinning them (they keep tracking future global changes). " - "Use after deliberately changing the default model." - ), - ) - cron_resnap.add_argument( - "job_id", nargs="?", help="Job ID to resnap (omit with --all)" - ) - cron_resnap.add_argument( - "--all", - action="store_true", - help="Resnap every unpinned agent job to the current global resolution", - ) - # cron status cron_subparsers.add_parser("status", help="Check if cron scheduler is running") diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 2a57daa3d4..288404f87e 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1600,7 +1600,6 @@ _PLUGIN_COMPAT_LAZY = { 'apply_whatsapp_onboarding': ('hermes_cli.web_routers.messaging', 'apply_whatsapp_onboarding'), 'approve_pairing': ('hermes_cli.web_routers.ops', 'approve_pairing'), 'auth_mcp_server': ('hermes_cli.web_routers.mcp', 'auth_mcp_server'), - 'build_cron_model_impact': ('hermes_cli.config', 'build_cron_model_impact'), 'bulk_delete_sessions_endpoint': ('hermes_cli.web_routers.sessions', 'bulk_delete_sessions_endpoint'), 'cancel_oauth_session': ('hermes_cli.web_routers.oauth', 'cancel_oauth_session'), 'cancel_telegram_onboarding': ('hermes_cli.web_routers.messaging', 'cancel_telegram_onboarding'), @@ -1789,7 +1788,6 @@ _PLUGIN_COMPAT_LAZY = { 'replace_mcp_servers': ('hermes_cli.web_routers.mcp', 'replace_mcp_servers'), 'rescan_dashboard_plugins': ('hermes_cli.web_routers.dashboard_ui', 'rescan_dashboard_plugins'), 'reset_memory': ('hermes_cli.web_routers.ops', 'reset_memory'), - 'resolve_cron_model_drift_defaults': ('hermes_cli.config', 'resolve_cron_model_drift_defaults'), 'resolve_gateway_liveness': ('gateway.status', 'resolve_gateway_liveness'), 'restart_gateway': ('hermes_cli.web_routers.actions', 'restart_gateway'), 'resume_cron_job': ('hermes_cli.web_routers.cron', 'resume_cron_job'), diff --git a/hermes_cli/web_server_config.py b/hermes_cli/web_server_config.py index a33f2171bc..1199a51efc 100644 --- a/hermes_cli/web_server_config.py +++ b/hermes_cli/web_server_config.py @@ -9,12 +9,10 @@ from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING from agent.model_metadata import is_local_endpoint from hermes_cli.config import ( DEFAULT_CONFIG, - build_cron_model_impact, cfg_get, clear_model_endpoint_credentials, find_provider_entry, read_raw_config, - resolve_cron_model_drift_defaults, ) from hermes_cli.web_server_memory import _normalize_memory_provider_name @@ -659,21 +657,6 @@ def _stale_aux_pins(cfg: dict, new_provider: str) -> list: return stale_aux -def _cron_model_impact(cfg: dict, provider: str, model: str) -> Any: - from hermes_cli.config import load_config - try: - effective_config = load_config() - effective_provider, effective_model = resolve_cron_model_drift_defaults(effective_config) - return 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) - return build_cron_model_impact(config=cfg, jobs={}) - - def _provider_entry(cfg: dict, provider: str) -> Any: providers_cfg = cfg.get("providers") return providers_cfg.get(provider) if isinstance(providers_cfg, dict) else None @@ -719,7 +702,6 @@ def _apply_main_assignment_sync(cfg: dict, provider: str, model: str, base_url: "base_url": model_cfg.get("base_url", ""), "gateway_tools": gateway_tools, "stale_aux": _stale_aux_pins(cfg, new_provider), - "cron_model_impact": _cron_model_impact(cfg, provider, model), } diff --git a/tests/cron/test_cron_provider_pin.py b/tests/cron/test_cron_provider_pin.py index cd3d90a6c6..dcab5bfb46 100644 --- a/tests/cron/test_cron_provider_pin.py +++ b/tests/cron/test_cron_provider_pin.py @@ -1,20 +1,14 @@ -"""Unpinned cron jobs run on their creation snapshot (#44585 follow-up). +"""Unpinned cron jobs run on the main agent model at fire time; ``pinned`` locks it. -Background: an UNPINNED cron job used to follow the live global default provider/model. A -temporary switch to a paid provider made every unpinned job silently inherit it on its next -tick (the $7.73 incident). The first fix failed closed on any drift, which instead killed every -unpinned job whenever the operator changed models — silently, for days. - -Current contract: - - create_job() snapshots the provider/model resolution WOULD pick at creation into - job["provider_snapshot"] / job["model_snapshot"] (unpinned, agent-backed jobs only). - - run_job() treats the snapshot as the effective pin: an unpinned axis runs on its snapshot - even after the global default moved. Explicit per-job pins and the cron.model / - cron.model_provider fleet defaults still win; a job with no snapshot follows the global - default as before. +Contract: + - run_job() resolves per-job pin > cron.model / cron.model_provider > the main agent model + (config ``model:``). There is no creation-time snapshot axis any more: a record that still + carries legacy ``provider_snapshot`` / ``model_snapshot`` keys follows the main model. + - create_job(pinned=True) / update_job({"pinned": True}) lock the CURRENT main provider+model + onto the job as an ordinary per-job pin; ``pinned=False`` releases both. These tests exercise the full run_job path (real imports, mocked AIAgent + -resolve_runtime_provider against a temp HERMES_HOME) and the create_job snapshot capture. +resolve_runtime_provider against a temp HERMES_HOME) and the job-store pin helpers. """ import sys @@ -34,7 +28,6 @@ def _base_job(**overrides): "prompt": "hello", "model": None, "provider": None, - "provider_snapshot": None, "base_url": None, } job.update(overrides) @@ -91,52 +84,32 @@ def _run(job, tmp_path, *, current_provider="openrouter", current_model=None, cr return success, error, agent_kwargs, (resolve_kwargs or None) -class TestSnapshotIsTheEffectivePin: - def test_unpinned_job_runs_on_snapshot_after_global_default_moved(self, tmp_path): - """Global default moved old-provider/old-model -> new-provider/new-model; the unpinned job - still runs, on what it was created under. Neither a skip nor a silent inherit.""" +class TestUnpinnedJobsFollowTheMainModel: + def test_legacy_snapshot_record_follows_the_main_model(self, tmp_path): + """A record created under the old snapshot design keeps running, on the CURRENT main + provider/model, never on what it was created under.""" job = _base_job(provider_snapshot="old-provider", model_snapshot="old-model") success, error, agent_kwargs, resolve_kwargs = _run( job, tmp_path, current_provider="new-provider", current_model="new-model") - assert success is True, error - assert agent_kwargs["model"] == "old-model" - assert resolve_kwargs["requested"] == "old-provider" - assert resolve_kwargs["target_model"] == "old-model" - - def test_explicit_job_pin_beats_snapshot(self, tmp_path): - job = _base_job( - provider="pinned-provider", model="pinned-model", - provider_snapshot="old-provider", model_snapshot="old-model") - success, error, agent_kwargs, resolve_kwargs = _run( - job, tmp_path, current_provider="new-provider", current_model="new-model", - cron_model="fleet-model") - - assert success is True, error - assert agent_kwargs["model"] == "pinned-model" - assert resolve_kwargs["requested"] == "pinned-provider" - - def test_cron_fleet_default_beats_snapshot(self, tmp_path): - """cron.model / cron.model_provider deliberately route the whole unpinned fleet.""" - job = _base_job(provider_snapshot="old-provider", model_snapshot="old-model") - success, error, agent_kwargs, resolve_kwargs = _run( - job, tmp_path, current_provider="new-provider", current_model="new-model", - cron_model="fleet-model", cron_model_provider="fleet-provider") - - assert success is True, error - assert agent_kwargs["model"] == "fleet-model" - assert resolve_kwargs["requested"] == "fleet-provider" - - def test_job_without_snapshot_follows_global_default(self, tmp_path): - """Legacy record (keys absent) keeps tracking the live global default.""" - job = _base_job() - job.pop("provider_snapshot", None) - success, error, agent_kwargs, resolve_kwargs = _run( - job, tmp_path, current_provider="new-provider", current_model="new-model") - assert success is True, error assert agent_kwargs["model"] == "new-model" assert resolve_kwargs["requested"] is None + assert resolve_kwargs["target_model"] == "new-model" + + def test_explicit_pin_then_fleet_default_beat_the_main_model(self, tmp_path): + pinned = _base_job(provider="pinned-provider", model="pinned-model") + success, error, agent_kwargs, resolve_kwargs = _run( + pinned, tmp_path, current_provider="new-provider", current_model="new-model", + cron_model="fleet-model", cron_model_provider="fleet-provider") + assert success is True, error + assert (agent_kwargs["model"], resolve_kwargs["requested"]) == ("pinned-model", "pinned-provider") + + success, error, agent_kwargs, resolve_kwargs = _run( + _base_job(), tmp_path, current_provider="new-provider", current_model="new-model", + cron_model="fleet-model", cron_model_provider="fleet-provider") + assert success is True, error + assert (agent_kwargs["model"], resolve_kwargs["requested"]) == ("fleet-model", "fleet-provider") def test_missing_model_guides_to_user_owned_cli(self, tmp_path, monkeypatch): """A missing-model failure cannot advertise agent-owned pinning.""" @@ -150,66 +123,59 @@ class TestSnapshotIsTheEffectivePin: assert "cronjob action=update" not in error -class TestCreateJobSnapshot: - """create_job captures provider_snapshot for unpinned agent jobs only.""" +class TestPinnedLocksTheMainModel: + """``pinned`` is a lock on the main model at the time it is set, stored as a plain pin.""" @staticmethod - def _isolate_storage(monkeypatch): - """Patch cron.jobs storage so create_job never touches the real store.""" - import contextlib + def _store(monkeypatch, tmp_path, main_model="main-model", main_provider="openrouter"): import cron.jobs as jobs + (tmp_path / "config.yaml").write_text(f"model:\n default: {main_model}\n") + monkeypatch.setattr(jobs, "get_hermes_home", lambda: tmp_path, raising=True) + state = {"jobs": []} + monkeypatch.setattr(jobs, "load_jobs", lambda: list(state["jobs"]), raising=True) + monkeypatch.setattr(jobs, "save_jobs", lambda j: state.__setitem__("jobs", list(j)), raising=True) + monkeypatch.setattr(jobs, "resolve_job_ref", lambda ref: next( + (j for j in state["jobs"] if j["id"] == ref), None), raising=True) + resolver = MagicMock(return_value={"provider": main_provider}) + monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", resolver) + return jobs, resolver - @contextlib.contextmanager - def _noop_lock(): - yield + def test_pinned_true_locks_then_pinned_false_releases(self, monkeypatch, tmp_path): + jobs, _ = self._store(monkeypatch, tmp_path) - monkeypatch.setattr(jobs, "_jobs_lock", _noop_lock, raising=True) - monkeypatch.setattr(jobs, "load_jobs", lambda: [], raising=True) - monkeypatch.setattr(jobs, "save_jobs", lambda j: None, raising=True) - return jobs + from tools.cronjob_job_args import _format_job - def test_unpinned_job_captures_snapshot(self, monkeypatch): - jobs = self._isolate_storage(monkeypatch) + unpinned = jobs.create_job(prompt="do a thing", schedule="every 1 hour") + assert (unpinned["model"], unpinned["provider"]) == (None, None) + assert _format_job(unpinned)["pinned"] is False - with patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - job = jobs.create_job(prompt="do a thing", schedule="every 1 hour") + locked = jobs.update_job(unpinned["id"], {"pinned": True}) + assert (locked["model"], locked["provider"]) == ("main-model", "openrouter") + assert _format_job(locked)["pinned"] is True + assert "pinned" not in jobs.load_jobs()[0] # derived, never stored - assert job["provider"] is None - assert job["provider_snapshot"] == "openrouter" + # The main model moves on; the locked job does not. + (tmp_path / "config.yaml").write_text("model:\n default: newer-model\n") + assert jobs.update_job(locked["id"], {"name": "renamed"})["model"] == "main-model" - def test_pinned_job_skips_snapshot(self, monkeypatch): - jobs = self._isolate_storage(monkeypatch) + released = jobs.update_job(locked["id"], {"pinned": False}) + assert (released["model"], released["provider"]) == (None, None) - resolver = MagicMock(return_value={"provider": "openrouter"}) - with patch("hermes_cli.runtime_provider.resolve_runtime_provider", resolver): - job = jobs.create_job( - prompt="do a thing", schedule="every 1 hour", provider="nous" - ) + def test_pinned_never_overrides_an_explicit_model(self, monkeypatch, tmp_path): + jobs, resolver = self._store(monkeypatch, tmp_path) - # Explicit provider → pinned → no snapshot needed, and resolution skipped. - assert job["provider"] == "nous" - assert job["provider_snapshot"] is None + job = jobs.create_job(prompt="do a thing", schedule="every 1 hour", model="my-model", + provider="nous", pinned=True) + assert (job["model"], job["provider"]) == ("my-model", "nous") resolver.assert_not_called() - def test_snapshot_resolution_error_fails_open_to_none(self, monkeypatch): - """If resolution raises at creation, snapshot is None — creation never breaks.""" - jobs = self._isolate_storage(monkeypatch) - - with patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - side_effect=RuntimeError("no creds"), - ): - job = jobs.create_job(prompt="do a thing", schedule="every 1 hour") - - assert job["provider_snapshot"] is None + still = jobs.update_job(job["id"], {"pinned": True, "model": "other-model"}) + assert still["model"] == "other-model" class TestRuntimeResolutionTargetModel: """run_job must resolve the primary provider against the model the job will actually run - (per-job pin > cron.model > snapshot > config default), so providers with model-specific + (per-job pin > cron.model > the main agent model), so providers with model-specific api_mode routing pick the mode for that model instead of the stale persisted default.""" def test_primary_resolution_passes_effective_model(self, tmp_path): @@ -220,134 +186,3 @@ class TestRuntimeResolutionTargetModel: assert success is True, error assert resolve_kwargs["target_model"] == "my-pinned-model" assert resolve_kwargs["requested"] == "openrouter" - - -class TestResnapshot: - """resnapshot_job / resnapshot_all_unpinned — 'adopt the current global - default without pinning' (#44585 companion). These refresh an unpinned - job's snapshot(s) to the CURRENT global resolution while leaving the job - unpinned, so it keeps tracking future global changes.""" - - @staticmethod - def _install_store(monkeypatch, initial_jobs): - """Install an in-memory cron job store backed by a real list so - resnapshot functions can load/save against it.""" - import contextlib - import cron.jobs as jobs - - store = [dict(j) for j in initial_jobs] # deep-ish copy per job - - @contextlib.contextmanager - def _lock(): - yield - - monkeypatch.setattr(jobs, "_jobs_lock", _lock, raising=True) - monkeypatch.setattr(jobs, "load_jobs", lambda: [dict(j) for j in store], raising=True) - - def _save(job_list): - store[:] = [dict(j) for j in job_list] - - monkeypatch.setattr(jobs, "save_jobs", _save, raising=True) - return jobs, store - - def _make_job(self, job_id, **overrides): - job = { - "id": job_id, - "name": f"job {job_id}", - "prompt": "do a thing", - "model": None, - "provider": None, - "model_snapshot": "old-model", - "provider_snapshot": "old-provider", - "base_url": None, - "no_agent": False, - } - job.update(overrides) - return job - - def test_resnapshot_unpinned_refreshes_to_current(self, monkeypatch, tmp_path): - jobs_mod, store = self._install_store( - monkeypatch, [self._make_job("j1", model_snapshot="old-model")] - ) - (tmp_path / "config.yaml").write_text("model:\n default: new-model\n") - monkeypatch.setattr("cron.jobs.get_hermes_home", lambda: tmp_path, raising=True) - with patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - updated = jobs_mod.resnapshot_job("j1") - - assert updated is not None - assert updated["model"] is None, "job must stay unpinned" - assert updated["model_snapshot"] == "new-model" - assert updated["provider_snapshot"] == "openrouter" - # Persisted too. - assert store[0]["model_snapshot"] == "new-model" - assert store[0]["provider_snapshot"] == "openrouter" - - def test_resnapshot_pinned_job_keeps_none_snapshot(self, monkeypatch, tmp_path): - # A fully-pinned job already carries None snapshots; resnapping must not - # clobber them into a global default. - jobs_mod, store = self._install_store( - monkeypatch, - [ - self._make_job( - "j1", - model="my-pinned-model", - provider="openrouter", - model_snapshot=None, - provider_snapshot=None, - ) - ], - ) - (tmp_path / "config.yaml").write_text("model:\n default: new-model\n") - monkeypatch.setattr("cron.jobs.get_hermes_home", lambda: tmp_path, raising=True) - with patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - updated = jobs_mod.resnapshot_job("j1") - - # Pinned axes stay pinned: model unchanged, snapshots still None. - assert updated["model"] == "my-pinned-model" - assert updated["model_snapshot"] is None - assert updated["provider_snapshot"] is None - - def test_resnapshot_missing_job_returns_none(self, monkeypatch, tmp_path): - jobs_mod, _store = self._install_store(monkeypatch, []) - assert jobs_mod.resnapshot_job("nope") is None - - def test_resnapshot_all_skips_no_agent_and_fully_pinned(self, monkeypatch, tmp_path): - jobs_mod, store = self._install_store( - monkeypatch, - [ - # unpinned, model-only → should be refreshed (provider axis too) - self._make_job("j1", model_snapshot="old", provider_snapshot="old"), - # no_agent → skipped entirely - self._make_job("j2", no_agent=True, model_snapshot="old", provider_snapshot="old"), - # fully pinned → skipped (nothing unpinned) - self._make_job( - "j3", - model="pm", provider="pp", - model_snapshot=None, provider_snapshot=None, - ), - ], - ) - (tmp_path / "config.yaml").write_text("model:\n default: new-model\n") - monkeypatch.setattr("cron.jobs.get_hermes_home", lambda: tmp_path, raising=True) - with patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - updated = jobs_mod.resnapshot_all_unpinned() - - ids = [j["id"] for j in updated] - assert ids == ["j1"], "only the unpinned agent job is refreshed" - by_id = {j["id"]: j for j in store} - assert by_id["j1"]["model_snapshot"] == "new-model" - assert by_id["j1"]["provider_snapshot"] == "openrouter" - # no_agent job keeps its (irrelevant) old snapshot untouched. - assert by_id["j2"]["model_snapshot"] == "old" - # pinned job keeps None. - assert by_id["j3"]["model_snapshot"] is None - diff --git a/tests/cron/test_cron_reasoning_effort.py b/tests/cron/test_cron_reasoning_effort.py index d4eb672b2b..bf0431f30d 100644 --- a/tests/cron/test_cron_reasoning_effort.py +++ b/tests/cron/test_cron_reasoning_effort.py @@ -96,14 +96,6 @@ class TestJobStoreReasoningEffort: update_job(job["id"], {"reasoning_effort": "warp9"}) assert load_jobs()[0]["reasoning_effort"] == "high" - def test_effort_change_does_not_trigger_snapshot_recompute(self, tmp_cron_dir): - """Effort is NOT an inference-snapshot axis: updating it alone must not - touch provider_snapshot/model_snapshot.""" - job = _create() - before = (job.get("provider_snapshot"), job.get("model_snapshot")) - updated = update_job(job["id"], {"reasoning_effort": "low"}) - assert (updated.get("provider_snapshot"), updated.get("model_snapshot")) == before - class TestSchedulerJobReasoningPrecedence: """Contract for cron/scheduler.py::_resolve_job_reasoning_config.""" diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index e0745909dd..afab9c477c 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -1275,8 +1275,8 @@ class TestRunJobConfigEnvVarExpansion: "id": "auth-fallback", "name": "auth fallback", "prompt": "hi", - "provider_snapshot": "openai-codex", - "model_snapshot": "gpt-5.6-sol", + "provider": "openai-codex", + "model": "gpt-5.6-sol", } fake_db = MagicMock() requested = [] @@ -1284,7 +1284,6 @@ class TestRunJobConfigEnvVarExpansion: def resolve_runtime(**kwargs): requested.append(kwargs.get("requested")) if kwargs.get("requested") == "openai-codex": - # The unpinned job's provider_snapshot is its effective pin. raise AuthError("No Codex credentials stored") assert kwargs["requested"] == "openrouter" assert kwargs["target_model"] == "z-ai/glm-5.2" diff --git a/tests/hermes_cli/test_cli_save_config_value.py b/tests/hermes_cli/test_cli_save_config_value.py index 75039d9355..d1f0a3ef52 100644 --- a/tests/hermes_cli/test_cli_save_config_value.py +++ b/tests/hermes_cli/test_cli_save_config_value.py @@ -47,22 +47,6 @@ class TestSaveConfigValueAtomic: result = yaml.safe_load(config_env.read_text()) assert result["auxiliary"]["compression"]["model"] == "google/gemini-3-flash-preview" - - - def test_model_write_runs_shared_cron_drift_warning(self, config_env, monkeypatch): - warning = MagicMock() - monkeypatch.setattr( - "hermes_cli.config.warn_unpinned_cron_jobs_after_model_config_change", - warning, - ) - - from cli import save_config_value - - assert save_config_value("model.default", "new-model") is True - warning.assert_called_once_with("model.default", "new-model") - - - def test_file_not_truncated_on_error(self, config_env, monkeypatch): """If atomic_yaml_write raises, the original file is untouched.""" original_content = config_env.read_text() diff --git a/tests/hermes_cli/test_cron_model_impact.py b/tests/hermes_cli/test_cron_model_impact.py deleted file mode 100644 index ef097414f7..0000000000 --- a/tests/hermes_cli/test_cron_model_impact.py +++ /dev/null @@ -1,177 +0,0 @@ -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_snapshot_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_cover_only_their_own_axis() -> 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"] - - -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, - "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, - "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_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_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index ced185a078..b4cae00563 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -377,47 +377,6 @@ class TestListNavigation: # Unpinned-cron notice on a global model change (#59031, #44585) # --------------------------------------------------------------------------- -def _write_cron_jobs(tmp_path, jobs): - cron_dir = tmp_path / "cron" - cron_dir.mkdir(parents=True, exist_ok=True) - (cron_dir / "jobs.json").write_text( - json.dumps({"jobs": jobs}), - encoding="utf-8", - ) - - -class TestCronModelChangeNotice: - """A global model change tells the operator which unpinned jobs stay on their snapshot.""" - - def test_notice_says_jobs_keep_running_and_names_the_user_owned_pin_path( - self, - _isolated_hermes_home, - capsys, - ): - _write_cron_jobs( - _isolated_hermes_home, - [ - { - "id": "model-drift-job", - "enabled": True, - "model": None, - "model_snapshot": "old-model", - } - ], - ) - - set_config_value("model.default", "new-model") - - notice = capsys.readouterr().out - assert "keeps running" in notice - assert "fail closed" not in notice - assert "hermes cron edit --provider --model " in notice - assert "cronjob action=update" not in notice - - -# --------------------------------------------------------------------------- -# String-typed config values — regression tests for #47515 -# --------------------------------------------------------------------------- class TestStringTypedConfigValues: @pytest.mark.parametrize("value", ["off", "on", "yes", "no", "true", "false", "01"]) diff --git a/tests/hermes_cli/test_web_server_cron_profiles.py b/tests/hermes_cli/test_web_server_cron_profiles.py index 5bbb4eb089..ef03ab6272 100644 --- a/tests/hermes_cli/test_web_server_cron_profiles.py +++ b/tests/hermes_cli/test_web_server_cron_profiles.py @@ -974,102 +974,6 @@ async def test_dashboard_cron_rejects_missing_context_from(isolated_profiles): assert "missing-job-id" in update_exc.value.detail - - - - -@pytest.mark.asyncio -async def test_dashboard_cron_noop_inference_fields_keep_existing_snapshots( - isolated_profiles, - monkeypatch, -): - from hermes_cli import runtime_provider, web_server - - current_provider = {"name": "initial-provider"} - monkeypatch.setattr( - runtime_provider, - "resolve_runtime_provider", - lambda **kwargs: {"provider": current_provider["name"]}, - ) - - job = _web_server_cron._call_cron_for_profile( - "worker_alpha", - "create_job", - prompt="managed by named profile", - schedule="every 1h", - name="dashboard-edit-job", - ) - - assert job["provider_snapshot"] == "initial-provider" - assert job["model_snapshot"] == "test-model" - - current_provider["name"] = "changed-provider" - (isolated_profiles["worker_alpha"] / "config.yaml").write_text( - "model: changed-model\n", - encoding="utf-8", - ) - - updated = await _rt_cron.update_cron_job( - job["id"], - _web_models.CronJobUpdate( - updates={ - "name": "dashboard-edit-job-renamed", - "provider": None, - "model": None, - "base_url": None, - "no_agent": False, - } - ), - profile="worker_alpha", - ) - - assert updated["name"] == "dashboard-edit-job-renamed" - assert updated["provider_snapshot"] == "initial-provider" - assert updated["model_snapshot"] == "test-model" - - -@pytest.mark.asyncio -async def test_update_cron_job_clears_snapshots_for_no_agent( - isolated_profiles, - monkeypatch, -): - from hermes_cli import runtime_provider, web_server - - monkeypatch.setattr( - runtime_provider, - "resolve_runtime_provider", - lambda **kwargs: {"provider": "worker-provider"}, - ) - scripts_dir = isolated_profiles["worker_alpha"] / "scripts" - scripts_dir.mkdir() - (scripts_dir / "collect.py").write_text("print('ok')\n", encoding="utf-8") - - job = _web_server_cron._call_cron_for_profile( - "worker_alpha", - "create_job", - prompt="managed by named profile", - schedule="every 1h", - name="agent-to-script-job", - ) - - assert job["provider_snapshot"] == "worker-provider" - assert job["model_snapshot"] == "test-model" - - updated = await _rt_cron.update_cron_job( - job["id"], - _web_models.CronJobUpdate( - updates={ - "script": str(scripts_dir / "collect.py"), - "no_agent": True, - } - ), - profile="worker_alpha", - ) - - assert updated["provider_snapshot"] is None - assert updated["model_snapshot"] is None - - @pytest.mark.asyncio async def test_update_cron_job_rejects_id_mutation(isolated_profiles, monkeypatch): """Dashboard surfaces a 400 (not a 500 or silent rename) when an diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index a1f207a646..42e7d36b22 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -79,8 +79,6 @@ class TestProfileScopedConfig: assert _cfg(isolated_profiles["worker_beta"]).get("timezone") == "Pluto/Far" assert _cfg(isolated_profiles["default"]).get("timezone") != "Pluto/Far" - - def test_unknown_profile_404(self, client, isolated_profiles): resp = client.get("/api/config", params={"profile": "ghost"}) assert resp.status_code == 404 @@ -139,8 +137,6 @@ class TestProfileScopedMcp: "mcp_servers", {} ) - - def test_mcp_test_oauth_server_without_token_is_not_ok( self, client, isolated_profiles, monkeypatch ): @@ -363,115 +359,6 @@ class TestProfileScopedModel: assert resp.status_code == 200 and resp.json()["model_set"] is False assert "Unknown provider 'nobox'" in resp.json()["model_error"] - 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, - "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() - - - - def test_model_options_uses_config_only_scope_for_selected_profile( self, client, monkeypatch @@ -972,8 +859,6 @@ class TestProfileScopedAudio: #64057). """ - - def test_transcribe_runs_inside_target_profile_home( self, client, isolated_profiles, monkeypatch ): diff --git a/tests/tools/test_cronjob_tools.py b/tests/tools/test_cronjob_tools.py index b80dd0e7e5..2f06eff126 100644 --- a/tests/tools/test_cronjob_tools.py +++ b/tests/tools/test_cronjob_tools.py @@ -654,45 +654,6 @@ class TestLocalDeliveryNotice: assert created["deliver"] == "origin" assert "local-only cron job" not in created["message"] - def test_resnap_requires_scope(self): - # resnap with neither job_id nor all=true must refuse to guess scope. - result = json.loads(cronjob(action="resnap")) - assert result["success"] is False - assert "resnap requires either" in result["error"] - - def test_resnap_single_job(self, monkeypatch, tmp_path): - from unittest.mock import patch as _patch - # Deterministic global resolution for the snapshot recompute. - (tmp_path / "config.yaml").write_text("model:\n default: new-model\n") - monkeypatch.setattr("cron.jobs.get_hermes_home", lambda: tmp_path, raising=True) - with _patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - created = json.loads( - cronjob(action="create", prompt="Check", schedule="every 1h") - ) - job_id = created["job_id"] - result = json.loads(cronjob(action="resnap", job_id=job_id)) - assert result["success"] is True - assert "remains unpinned" in result["message"] - assert result["job"]["job_id"] == job_id - - def test_resnap_all(self, monkeypatch, tmp_path): - from unittest.mock import patch as _patch - (tmp_path / "config.yaml").write_text("model:\n default: new-model\n") - monkeypatch.setattr("cron.jobs.get_hermes_home", lambda: tmp_path, raising=True) - with _patch( - "hermes_cli.runtime_provider.resolve_runtime_provider", - return_value={"provider": "openrouter"}, - ): - cronjob(action="create", prompt="One", schedule="every 1h") - cronjob(action="create", prompt="Two", schedule="every 2h") - result = json.loads(cronjob(action="resnap", all=True)) - assert result["success"] is True - assert "Refreshed inference snapshots on" in result["message"] - assert len(result["updated_jobs"]) >= 1 - class TestValidateCronBaseUrl: """The cron base_url guard must not let a NAMED custom provider's stored diff --git a/tools/cronjob_job_args.py b/tools/cronjob_job_args.py index be300b8d14..5308c65488 100644 --- a/tools/cronjob_job_args.py +++ b/tools/cronjob_job_args.py @@ -377,6 +377,8 @@ def _format_job(job: Dict[str, Any]) -> Dict[str, Any]: "prompt_preview": prompt[:100] + "..." if len(prompt) > 100 else prompt, "model": job.get("model"), "provider": job.get("provider"), + # Locked to its own model; unpinned jobs follow cron.model, then the main agent model. + "pinned": bool(str(job.get("model") or "").strip()), "base_url": job.get("base_url"), "schedule": job.get("schedule_display") or "?", "repeat": _repeat_display(job), diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 51307c352f..f9fc717fb4 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -41,8 +41,6 @@ from cron.jobs import ( pause_job, remove_job, resolve_job_ref, - resnapshot_all_unpinned, - resnapshot_job, resume_job, update_job) from tools.cronjob_prompt_scan import _scan_cron_prompt @@ -612,6 +610,7 @@ def _action_create(a: Dict[str, Any]) -> str: monitor_url=_normalize_optional_job_value(a["monitor_url"]), # CLI-only lane: absent from CRONJOB_SCHEMA and the model dispatch (models don't pick models). reasoning_effort=a["reasoning_effort"], + pinned=bool(a["pinned"]), failure_deliver=_resolve_cron_context_deliver(_normalize_deliver_param(a["failure_deliver"])), **({"paused": a["paused"], "paused_reason": a["paused_reason"]} if a["paused"] is not False or a["paused_reason"] is not None else {})) @@ -759,6 +758,8 @@ def _update_core_fields(job: Dict[str, Any], a: Dict[str, Any], updates: Dict[st updates["model"] = _normalize_optional_job_value(a["model"]) if a["provider"] is not None: updates["provider"] = _normalize_optional_job_value(a["provider"]) + if a["pinned"] is not None: + updates["pinned"] = bool(a["pinned"]) if a["base_url"] is not None: updates["base_url"] = _normalize_optional_job_value(a["base_url"], strip_trailing_slash=True) if a["reasoning_effort"] is not None: @@ -859,50 +860,7 @@ def _action_update(job: Dict[str, Any], a: Dict[str, Any]) -> str: {"success": True, "job": _format_job(updated)}, updated, _normalize_deliver_param(a["deliver"]))) -def _action_resnap(a: Dict[str, Any]) -> str: - """Adopt the current global inference resolution without pinning (#44585). - - Bulk (``all=true``) refreshes every unpinned job; single-job resolves - ``job_id`` and refreshes just that job. Refuses to guess scope. - """ - if bool(a["all"]): - updated = resnapshot_all_unpinned() - _notify_provider_jobs_changed_safe() - return _dumps({ - "success": True, - "message": ( - f"Refreshed inference snapshots on {len(updated)} unpinned " - "job(s) to the current global resolution. Jobs remain " - "unpinned and will track future global changes."), - "updated_jobs": [_format_job(j) for j in updated], - }) - job_id = a["job_id"] - if not job_id: - return tool_error( - "resnap requires either `job_id=` (single job) or `all=true` " - "(refresh every unpinned job). Refusing to guess scope.", - success=False, - ) - job, error = _resolve_job_or_error(job_id) - if error is not None: - return error - assert job is not None # error is None ⇔ job resolved - updated = resnapshot_job(job["id"]) - if not updated: - return tool_error(f"Failed to resnap job '{job_id}'", success=False) - _notify_provider_jobs_changed_safe() - return _dumps({ - "success": True, - "message": ( - f"Cron job '{updated['name']}' refreshed to the current " - "global inference resolution. It remains unpinned and will " - "track future global changes."), - "job": _format_job(updated), - }) - - -# Actions that need no job_id, and job-bound actions (job resolved first). -_JOBLESS_ACTIONS = {"create": _action_create, "list": _action_list, "resnap": _action_resnap} +_JOBLESS_ACTIONS = {"create": _action_create, "list": _action_list} _JOB_ACTIONS = { "remove": _action_remove, "update": _action_update, "run": _action_run, "run_now": _action_run, "trigger": _action_run, @@ -957,11 +915,11 @@ def cronjob( monitor_url: Optional[str] = None, reasoning_effort: Optional[str] = None, failure_deliver: Optional[Union[str, List[str]]] = None, - all: Optional[bool] = None, task_id: str = None, session_id: Optional[str] = None, paused: bool = False, - paused_reason: Optional[str] = None) -> str: + paused_reason: Optional[str] = None, + pinned: Optional[bool] = None) -> str: """Unified cron job management tool.""" a = dict(locals()) del a["task_id"] # unused but kept for handler signature compatibility @@ -1004,9 +962,9 @@ CRONJOB_SCHEMA = { "name": "cronjob_manage", "description": """Manage scheduled cron jobs: action='create' schedules a job from a prompt and/or skills; 'list' inspects jobs; 'update'/'pause'/'resume'/'remove' manage one by job_id (always list first — never guess job IDs); 'run' fires a job immediately in the BACKGROUND (returns a handle at once, outcome re-enters the conversation when done — do not wait or poll; optional 'prompt' adds transient context for that fire only). -'resnap' adopts the CURRENT global inference resolution for an unpinned job (job_id) or all unpinned jobs (all=true) WITHOUT pinning it, so it keeps tracking future global changes — use after deliberately changing the default model. +Jobs run on the main agent model (whatever `hermes model` is set to when they fire) unless pinned. -Jobs run in a fresh session with no current-chat context, so prompts must be self-contained, and the agent's FINAL RESPONSE is what gets delivered — cron runs are autonomous and cannot ask questions. Prefer updating an existing job over creating near-duplicates.""", +Jobs run in a fresh session with no current-chat context, so prompts must be self-contained, and the agent's FINAL RESPONSE is what gets delivered — cron runs are autonomous and cannot ask questions. Jobs run on the main agent model (whatever `hermes model` is set to when they fire) unless the user pins one. Prefer updating an existing job over creating near-duplicates.""", "parameters": { "type": "object", "properties": { @@ -1014,15 +972,15 @@ Jobs run in a fresh session with no current-chat context, so prompts must be sel "paused_reason": {"type": "string", "description": "Create only: auditable reason; requires paused=true."}, "action": { "type": "string", - "description": "One of: create, list, update, pause, resume, remove, run, resnap. When action=create, the 'schedule' and 'prompt' fields are REQUIRED. When action=resnap, pass either job_id (single job) or all=true (every unpinned job)." + "description": "One of: create, list, update, pause, resume, remove, run. When action=create, the 'schedule' and 'prompt' fields are REQUIRED." }, "job_id": { "type": "string", - "description": "Required for update/pause/resume/remove/run. For resnap: the job to adopt the current global inference resolution (omit if all=true)." + "description": "Required for update/pause/resume/remove/run." }, - "all": { + "pinned": { "type": "boolean", - "description": "Only for action='resnap'. all=true refreshes the inference snapshot of EVERY unpinned agent job to the current global resolution (bulk 'make everything follow my new default'). Must be explicitly set to true — never implied. Omit (or false) to resnap a single job via job_id." + "description": "For create/update. ONLY set when the user explicitly asks to pin (or unpin) a job's model. pinned=true locks the CURRENT main agent model (and its provider) onto the job so later `hermes model` / `/model` changes never touch it; pinned=false releases the lock so the job follows the main agent model again. Never set it on your own initiative: by default jobs follow the main model." }, "prompt": { "type": "string", @@ -1117,7 +1075,7 @@ def check_cronjob_requirements() -> bool: _HANDLER_FORWARDED_ARGS = ( "job_id", "prompt", "schedule", "name", "repeat", "deliver", "failure_deliver", "skill", "skills", "reason", "script", "context_from", "continuity", "enabled_toolsets", "workdir", "no_agent", "attach_to_session", - "paused_reason", "all") + "paused_reason", "pinned") def _cronjob_handler(args, **kw): diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index 69607d5599..e72fdd1ba3 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -23,11 +23,11 @@ Cron jobs can: All of this is available to Hermes itself through the `cronjob` tool, so you can create, pause, edit, and remove jobs by asking in plain language — no CLI required. :::tip -**Which model does a cron job run on?** Resolution at fire time is: per-job pin → `cron.model` in `config.yaml` → the global default from `hermes model`. +**Which model does a cron job run on?** Resolution at fire time is: per-job pin → `cron.model` in `config.yaml` → the main agent model from `hermes model`. -- **Per-job pin** — set by *you* via the dashboard, `hermes cron create/edit --model … --provider …`, or by editing `~/.hermes/cron/jobs.json`. Once set, it sticks until you change it. The agent's `cronjob` tool cannot set or change per-job models — inference pins are user-owned. +- **Per-job pin** — a job that carries its own model. Set it to a specific model via the dashboard, `hermes cron create/edit --model … --provider …`, or by editing `~/.hermes/cron/jobs.json`; or **lock in the current main model** with `hermes cron create/edit --pin` (the agent's `cronjob` tool can do this too with `pinned=true`, but only when you ask it to). `--unpin` (`pinned=false`) releases the lock. The agent cannot point a job at a *different* model — inference pins are user-owned. - **`cron.model` / `cron.model_provider`** — a cron-fleet default: every unpinned job runs on this model, independent of your chat model. Set it once (`hermes config set cron.model `) and switching your chat model with `hermes model` or `/model` never touches your cron fleet. -- **Global default** — only when neither of the above is set does a job follow `hermes model`. Hermes **snapshots** the provider and model at creation, and that snapshot is the job's effective pin: if you later switch the global default (`hermes model`, `/model`, `hermes config set model.default …`), the job **keeps running on the model and provider it was created under** and logs one INFO line per run noting the difference. A global model change never stops a scheduled job, and an unattended job never silently inherits a switch to a paid provider/model (#44585). To move a job to the new default, **resnap** it (`hermes cron resnap `, or `--all` for every unpinned job) so it adopts the current default while staying unpinned, pin it (`hermes cron edit --provider --model `), or set `cron.model` to move the whole fleet at once. Jobs created before snapshots existed keep following the live global default. +- **Main agent model** — when neither of the above is set, a job runs on whatever `hermes model` / `/model` is set to **at the moment it fires**. Change your main model and every unpinned job follows on its next run. Whichever provider a job resolves to, its provider-specific request settings (e.g. `request_overrides` such as `extra_body`/`extra_headers` for custom providers) carry into the scheduled run just like an interactive session. @@ -119,28 +119,17 @@ Or: `hermes config set cron.preflight false` ## Moving unpinned jobs to a new global default -An unpinned job stays on the provider/model it was created under, so changing your chat model -never changes (or stops) your cron fleet. When you *do* want scheduled jobs to move: +An unpinned job follows the main agent model, so `hermes model` moves your cron fleet with it. +When you want a job to *stay* on a model: ```bash -hermes cron edit --provider --model # one job -hermes config set cron.model # every unpinned job +hermes cron edit --pin # lock the current main model onto one job +hermes cron edit --provider --model # pin an explicit model +hermes cron edit --unpin # follow the main model again +hermes config set cron.model # every unpinned job, without touching chat ``` -`hermes config set model.default …` and the Desktop model picker list the unpinned jobs that will -keep their original model so you can decide deliberately. Stored snapshots are refreshed whenever -you edit a job's provider, model, or base URL. - -Resnapping refreshes an unpinned job's stored snapshot to the current global resolution without -pinning it, so it keeps tracking future changes: - -```bash -hermes cron resnap # one job -hermes cron resnap --all # every unpinned agent job -``` - -The agent-facing `cronjob` tool accepts the same action (`action=resnap job_id=` or -`action=resnap all=true`). Pinned axes and `no_agent` script jobs are left untouched. +`hermes cron list` and the `cronjob` tool report `pinned` per job. ## Skill-backed cron jobs