diff --git a/AGENTS.md b/AGENTS.md index 314e0d97f7..d2846c1558 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -364,7 +364,8 @@ 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 (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/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/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index d4eba9eafa..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,6 +4,7 @@ import { useNavigate } from 'react-router' import { blurComposerInput } from '@/app/chat/composer/focus' import { useComposerSurfaceId } from '@/app/chat/composer/scope' +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' @@ -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/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..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,7 +1,11 @@ +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, 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' import { invalidateSkillSuggestionIndex } from '@/store/suggestion-providers/skill' @@ -68,7 +72,31 @@ export function handleToolEvent(ctx: GatewayEventContext): boolean { if (event.type === 'tool.complete') { if (sessionId) { flushQueuedDeltas(sessionId) + + 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 { previewTarget, status } = toolPreviewOutcome({ + type: 'tool-call', + toolName: payload.name, + args: payload.args ?? {}, + result: payload.result, + toolResultMetadata: payload, + completedAt: occurredAt + }) + + if (status === 'success' && previewTarget && isPreviewableTarget(previewTarget)) { + const record = pendingProduction ? reofferPreviewArtifact : recordPreviewArtifact + record(sessionId, previewTarget, state?.cwd ?? '', storedSessionIdForRuntimeId(sessionId) ?? sessionId) + } + } + // 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/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..a7fc3638e3 --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/steering-session.ts @@ -0,0 +1,97 @@ +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 + + if (!sessionId) { + return null + } + + const selectedStoredSessionId = selectedStoredSessionIdRef.current + const routedStoredSessionId = getRoutedStoredSessionId() + const bindings = runtimeIdByStoredSessionIdRef.current + const boundStoredSessionId = findStoredIdForRuntimeId(bindings, sessionId) + const sessions = $sessions.get() + + const matchesSelection = (id: string) => + Boolean(selectedStoredSessionId && idsShareLineage(id, selectedStoredSessionId, sessions)) + + // 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 + } + + // 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) + } + } + } +} 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/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index a0aaed80ca..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 @@ -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']) || @@ -1435,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/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..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 @@ -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,27 @@ 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), + $storedId: atom(null), 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 +49,7 @@ afterEach(() => { $previewStatusBySession.set({}) $activeSessionId.set(null) $currentCwd.set('') + $messages.set([]) }) describe('tool row preview recording', () => { @@ -68,9 +73,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 7aacf22101..875e6b0b3b 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -418,10 +418,16 @@ 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, + $messages: $sessionMessages + } = useSessionView() useEffect(() => { - if (isPending || !previewTarget || !isPreviewableTarget(previewTarget)) { + if (view.status !== 'success' || !previewTarget || !isPreviewableTarget(previewTarget)) { return } @@ -430,10 +436,12 @@ function ToolEntry({ part }: ToolEntryProps) { // or cwd change. const sessionId = $sessionRuntimeId.get() - if (sessionId) { - recordPreviewArtifact(sessionId, previewTarget, $sessionCwd.get() || '') + // 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() || '', $sessionStoredId.get() ?? sessionId) } - }, [$sessionCwd, $sessionRuntimeId, isPending, previewTarget]) + }, [$sessionCwd, $sessionRuntimeId, $sessionStoredId, $sessionMessages, messageId, previewTarget, view.status]) const detailSections = useMemo(() => { if (!view.detail) { diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 191c42b6e5..fc578caab9 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1754,20 +1754,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 9ba492020e..814f4be725 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2550,22 +2550,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 1216a50172..5cf4e87891 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -2063,22 +2063,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 37267b12e9..5e968f5204 100644 --- a/apps/desktop/src/i18n/ru.ts +++ b/apps/desktop/src/i18n/ru.ts @@ -2322,18 +2322,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 1503217221..44453368b7 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -2166,21 +2166,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 932e3c06d3..0463465a2b 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -2053,21 +2053,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 598db2ae4d..4599ebb54d 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2704,21 +2704,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/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/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/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..8979f02077 --- /dev/null +++ b/apps/desktop/src/store/preview-status-lifecycle.test.ts @@ -0,0 +1,65 @@ +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('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') + + 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() +}) + +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') + 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 e9ffbf322a..97adc0858a 100644 --- a/apps/desktop/src/store/preview-status.test.ts +++ b/apps/desktop/src/store/preview-status.test.ts @@ -1,13 +1,19 @@ 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(() => $previewStatusBySession.set({})) +beforeEach(() => { + window.localStorage.clear() + $previewStatusBySession.set({}) +}) describe('recordPreviewArtifact', () => { it('appends new targets newest-last and is idempotent', () => { @@ -38,4 +44,37 @@ describe('recordPreviewArtifact', () => { clearPreviewArtifacts('s1') expect($previewStatusBySession.get().s1).toBeUndefined() }) + + 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 618f06f7bd..9c44203e7e 100644 --- a/apps/desktop/src/store/preview-status.ts +++ b/apps/desktop/src/store/preview-status.ts @@ -1,6 +1,12 @@ import { atom } from 'nanostores' -import { previewName } from '@/lib/preview-targets' +import { previewArtifactKey, previewName } from '@/lib/preview-targets' +import { readJson, writeJson } from '@/lib/storage' + +import { normalizeProfileKey } from './profile' +import { ownerLookupSessionRows, resolveComposerSessionKey } from './session' +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) @@ -14,16 +20,284 @@ import { previewName } from '@/lib/preview-targets' 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>({}) +/** 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() + +/** Not JSON on purpose: it can never collide with an encoded owner scope. */ +const RUNTIME_SCOPE_PREFIX = 'runtime:' + +function runtimeScopeKey(runtimeId: string): string { + return RUNTIME_SCOPE_PREFIX + 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!) + } +} + +/** 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 + // Historical rows in background tiles do not belong to the active connection. + const owner = knownOwnerForSession(runtimeId) ?? knownOwnerForSession(storedId) + + if (!owner) { + return runtimeScopeKey(runtimeId) + } + + const route = isSessionOwnerRoute(owner) ? owner : null + const profile = normalizeProfileKey(isSessionOwnerRoute(owner) ? owner.profile : owner) + const targetProfile = normalizeProfileKey(route?.targetProfile || profile) + // 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 ?? 'local') + ) + + return encodeScope({ + connectionId, + 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) + + 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) + capMap(scopeByRuntime) + + return scope +} + +function readDismissedPreviewIds(): DismissedPreviewIds { + const value = readJson(DISMISSED_PREVIEWS_KEY) + + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return {} + } + + return Object.fromEntries( + Object.entries(value) + .slice(-MAX_DISMISSED_SESSIONS) + .flatMap(([scope, ids]) => { + if (!Array.isArray(ids) || scope.length > 4096 || !decodeScope(scope)) { + 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 ? [[scope, strings]] : [] + }) + ) +} + +function isDismissed(scope: string, id: string): boolean { + return Boolean(readDismissedPreviewIds()[scope]?.includes(id) || volatileDismissals.get(scope)?.includes(id)) +} + +/** 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 = update([...new Set([...(dismissed[scope] ?? []), ...(volatileDismissals.get(scope) ?? [])])]) + const { [scope]: _previous, ...rest } = dismissed + + const next = Object.fromEntries( + [...Object.entries(rest), ...(ids.length > 0 ? [[scope, ids.slice(-MAX_DISMISSED_TARGETS)]] : [])].slice( + -MAX_DISMISSED_SESSIONS + ) + ) + + if (ids.length > 0) { + volatileDismissals.set(scope, next[scope]) + capMap(volatileDismissals) + } else { + volatileDismissals.delete(scope) + } + + if (!isRuntimeScopeKey(scope)) { + writeJson(DISMISSED_PREVIEWS_KEY, next) + + if (ids.length > 0 && readDismissedPreviewIds()[scope]?.includes(ids[ids.length - 1])) { + volatileDismissals.delete(scope) + } + } +} + +function rememberDismissedPreview(scope: string, id: string): void { + if (!isDismissed(scope, id)) { + updateDismissed(scope, ids => [...ids, id]) + } +} + +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 { + 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]) => !isRuntimeScopeKey(key)))) + + 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 }] + }) + ) + } +} + +export function migratePreviewArtifactsForProfile(from: string, to: string): void { + rewriteDismissalScopes(key => { + const scope = decodeScope(key) + + if (!scope || (scope.connectionId && scope.connectionId !== 'local')) { + return key + } + + 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 { + const routeProfile = normalizeProfileKey(route?.profile) + const routeTarget = route?.targetProfile ? normalizeProfileKey(route.targetProfile) : '' + const routeConnection = (route?.connectionId ?? '').trim() + + rewriteDismissalScopes(key => { + const scope = decodeScope(key) + + if (!scope) { + return key + } + + const matches = route + ? 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 : key + }) +} + const writePreviews = (sid: string, items: PreviewArtifact[]) => { const current = $previewStatusBySession.get() @@ -39,32 +313,90 @@ 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 }) +} + +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 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) { +export function recordPreviewArtifact(sid: string, target: string, cwd: string, dismissalSid = sid) { const raw = target.trim() if (!sid || !raw) { return } - const list = $previewStatusBySession.get()[sid] ?? [] + const id = previewArtifactKey(raw, cwd) - if (list.some(item => item.id === raw)) { + if ($previewStatusBySession.get()[sid]?.some(item => item.id === id)) { return } - writePreviews(sid, [...list, { cwd, id: raw, label: previewName(raw), target: raw }].slice(-MAX_PER_SESSION)) + appendPreviewArtifact(sid, raw, cwd, id, reconcileDismissalScope(sid, dismissalSid)) } -export function dismissPreviewArtifact(sid: string, id: string) { +/** 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 + } + + 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) { + const current = reconcileDismissalScope(sid, dismissalSid) const list = $previewStatusBySession.get()[sid] + const scope = list?.find(item => item.id === id)?.dismissalScope ?? current if (list) { writePreviews( @@ -72,6 +404,8 @@ export function dismissPreviewArtifact(sid: string, id: string) { list.filter(item => item.id !== id) ) } + + rememberDismissedPreview(scope, id) } export function clearPreviewArtifacts(sid: string) { 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-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/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 3f7e5bf813..d5fe0268e7 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -1559,21 +1559,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 { @@ -1742,8 +1727,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/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/cli.py b/cli.py index f080f54665..1769efe5ae 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/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 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 diff --git a/cron/jobs.py b/cron/jobs.py index a549d8c0cd..c489a1842b 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 053913f7c8..242e8a2fd3 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 6e37b9985e..c6f7e78b38 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3129,172 +3129,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)) @@ -3802,7 +3636,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 63647f4ed4..4a43402d26 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -1754,9 +1754,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/tools_config_providers.py b/hermes_cli/tools_config_providers.py index e9abd1e900..09ffe32bfa 100644 --- a/hermes_cli/tools_config_providers.py +++ b/hermes_cli/tools_config_providers.py @@ -224,7 +224,13 @@ 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" 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 @@ -409,10 +415,14 @@ def _browser_provider_active(provider: dict, config: dict) -> bool: return True -def _browser_backend_active(provider: dict, config: dict) -> bool: +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") - if backend is False: - backend = "off" # YAML 1.1: unquoted `off` parses as boolean False + return "off" if backend is False else (backend or "") + + +def _browser_backend_active(provider: dict, config: dict) -> bool: + backend = _browser_backend(config) if backend == provider["browser_backend"]: return True if backend: diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index bde26f874b..6015a14276 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1609,7 +1609,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'), @@ -1798,7 +1797,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 f27bf40adb..f8efb8a981 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 from tools.wake_word import _PROVIDER_PREFERENCE @@ -664,21 +662,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 @@ -724,7 +707,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/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 8e2facc0d7..a0b9ac044f 100644 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -414,6 +414,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 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 efff400404..4aa41bf146 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 af8272421f..a8204e531a 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_post_setup_gating.py b/tests/hermes_cli/test_post_setup_gating.py index 4768e3ef80..9f1826d080 100644 --- a/tests/hermes_cli/test_post_setup_gating.py +++ b/tests/hermes_cli/test_post_setup_gating.py @@ -56,3 +56,78 @@ 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 + + +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 diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 658b0f26ff..d6f7d8d3c2 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 ebf99d4ec7..1b9f83a136 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/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 126f617b2d..78db26cb1b 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -397,6 +397,825 @@ class TestNativeEnvironmentContracts: assert hermes_win not in entries assert user_win in entries + def test_empty_pythonpath_unchanged(self): + """An empty PYTHONPATH is a no-op (falsy -> early return).""" + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + env = {"PYTHONPATH": ""} + _strip_hermes_owned_pythonpath(env) + # Empty string is falsy, so the function returns early without + # modifying the dict. The key stays as-is (empty string). + assert env.get("PYTHONPATH") == "" + + def test_empty_component_preserved(self): + """An empty component means cwd and must survive unchanged.""" + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + + user_pp = os.pathsep.join(["/foo", "", "/bar"]) + env = {"PYTHONPATH": user_pp} + + _strip_hermes_owned_pythonpath(env) + + assert env["PYTHONPATH"] == user_pp + + def test_raw_user_spelling_preserved(self): + """The sanitizer does not trim, normalize, or deduplicate user entries.""" + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + + user_pp = os.pathsep.join([ + " /opt/user-lib ", + "relative/../lib", + "", + "/opt/user-lib", + "/opt/user-lib", + ]) + env = {"PYTHONPATH": user_pp} + + _strip_hermes_owned_pythonpath(env) + + assert env["PYTHONPATH"] == user_pp + + + def test_base_python_sanitizer_uses_validated_separate_runtime_venv(self, tmp_path, monkeypatch): + """A base interpreter strips the exact Windows runtime site-packages. + + This deliberately uses a synthetic Hermes venv separate from the test + runner: sys.prefix represents base Python, while validated VIRTUAL_ENV + identifies ``/venv`` as the Hermes runtime producer contract. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + + repo_root = tmp_path / "hermes-agent" + runtime_venv = repo_root / "venv" + runtime_sp = runtime_venv / "Lib" / "site-packages" + runtime_sp.mkdir(parents=True) + (runtime_venv / "pyvenv.cfg").write_text("version = 3.11\n", encoding="utf-8") + base_prefix = tmp_path / "base-python" + unrelated = "/custom/lib/python3.13/site-packages" + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", (repo_root,)) + monkeypatch.setattr(local, "_in_venv", False) + monkeypatch.setattr(local, "_hermes_site_packages", None) + monkeypatch.setattr(local.sys, "prefix", str(base_prefix)) + monkeypatch.setattr(local.sys, "base_prefix", str(base_prefix)) + + env = { + "VIRTUAL_ENV": str(runtime_venv), + "PYTHONPATH": os.pathsep.join([str(runtime_sp), unrelated]), + } + result = local._sanitize_subprocess_env(env) + + assert Path(local.sys.prefix) == base_prefix + assert runtime_venv != Path(local.sys.prefix) + assert result["PYTHONPATH"] == unrelated + assert "VIRTUAL_ENV" not in result + + def test_unrelated_virtual_env_is_not_runtime_provenance(self, tmp_path, monkeypatch): + """An arbitrary inherited VIRTUAL_ENV cannot claim PYTHONPATH ownership.""" + import tools.environments.local as local + from tools.environments import local_pythonpath + + repo_root = tmp_path / "hermes-agent" + repo_root.mkdir() + unrelated_venv = tmp_path / "user-venv" + unrelated_sp = unrelated_venv / "Lib" / "site-packages" + unrelated_sp.mkdir(parents=True) + (unrelated_venv / "pyvenv.cfg").write_text("version = 3.13\n", encoding="utf-8") + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", (repo_root,)) + monkeypatch.setattr(local, "_in_venv", False) + monkeypatch.setattr(local, "_hermes_site_packages", None) + + env = { + "VIRTUAL_ENV": str(unrelated_venv), + "PYTHONPATH": str(unrelated_sp), + } + local_pythonpath._strip_hermes_owned_pythonpath(env) + + assert env["PYTHONPATH"] == str(unrelated_sp) + + + def test_no_pythonpath_key(self): + """Missing PYTHONPATH key is a no-op.""" + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + env = {"PATH": "/usr/bin"} + _strip_hermes_owned_pythonpath(env) + assert "PYTHONPATH" not in env + + + @pytest.mark.parametrize("builder", [ + "_make_run_env", + "_sanitize_subprocess_env", + "hermes_subprocess_env", + ]) + def test_builders_strip_hermes_venv_pythonpath(self, builder): + """Every subprocess env builder applies the same sanitation contract: + Hermes venv site-packages is stripped, user entries survive. + """ + from tools.environments import local as local_mod + + venv_sp = str(_running_venv_site_packages()) + seed = { + "PATH": "/usr/bin:/bin", + "HOME": "/home/user", + "PYTHONPATH": os.pathsep.join([venv_sp, "/home/user/my-lib"]), + } + with patch.dict(os.environ, seed, clear=True): + if builder == "_make_run_env": + result = local_mod._make_run_env({}) + elif builder == "_sanitize_subprocess_env": + result = local_mod._sanitize_subprocess_env(dict(os.environ)) + else: + result = local_mod.hermes_subprocess_env() + pp = result.get("PYTHONPATH", "") + entries = pp.split(os.pathsep) if pp else [] + assert venv_sp not in entries + assert "/home/user/my-lib" in entries + + def test_scrub_child_env_strips_hermes_venv_pythonpath(self): + """execute_code's _scrub_child_env path: after scrubbing, Hermes venv + site-packages entries should be stripped when + _strip_hermes_owned_pythonpath is applied (as the spawn path does), + while user entries (even for another Python version) are preserved. + """ + from tools.code_execution_env import _scrub_child_env + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + + venv_sp = str(_running_venv_site_packages()) + other_sp = "/opt/other-venv/lib/python3.99/site-packages" + source = { + "PATH": "/usr/bin", + "HOME": "/home/user", + "PYTHONPATH": os.pathsep.join([venv_sp, other_sp, "/home/user/my-lib"]), + } + scrubbed = _scrub_child_env(source) + # The scrubber passes PYTHONPATH through (it's in _SAFE_ENV_PREFIXES). + assert "PYTHONPATH" in scrubbed + # Now apply the selective strip (as the spawn path does). + _strip_hermes_owned_pythonpath(scrubbed) + pp = scrubbed.get("PYTHONPATH", "") + entries = pp.split(os.pathsep) if pp else [] + assert venv_sp not in entries + assert other_sp in entries + assert "/home/user/my-lib" in entries + + @pytest.mark.parametrize("same_env", [True, False]) + def test_execute_code_composition_strips_inherited_hermes_entries(self, same_env): + """Integration: execute_code's real spawn path composes a clean PYTHONPATH. + + Seeds a contaminated inherited PYTHONPATH (Hermes repo root + Hermes + venv site-packages + user entries) through os.environ and drives + execute_code all the way to Popen. Proves the #84500 conditional + composition and the #82581 selective strip compose correctly: + + * inherited Hermes venv site-packages never survive into the sandbox; + * the staging tmpdir stays the first entry; + * the repo root is deliberately re-added exactly once for a same-env + child (the single occurrence proves the inherited copy was stripped + first) and stays absent for an external-environment child; + * user entries survive after the controlled entries. + """ + import tools.code_execution_tool as cet + from tools.code_execution_tool import execute_code + + def _mock_handle_function_call(function_name, function_args, task_id=None, user_task=None): + return '{"output": "mock", "exit_code": 0}' + + hermes_root = str(Path(cet.__file__).resolve().parents[1]) + venv_sp = str(_running_venv_site_packages()) + user_a = "/home/user/my-lib" + user_b = "/opt/project/lib" + captured = {} + + def _fake_popen(cmd, **kwargs): + 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 + return proc + + with patch("tools.code_execution_tool._load_config", + return_value={"mode": "strict"}), \ + patch("model_tools.handle_function_call", + side_effect=_mock_handle_function_call), \ + patch("tools.code_execution_env._uses_hermes_python_environment", + return_value=same_env), \ + patch("subprocess.Popen", side_effect=_fake_popen), \ + patch.dict(os.environ, { + "PYTHONPATH": os.pathsep.join( + [hermes_root, venv_sp, user_a, user_b]), + }): + execute_code(code="pass", task_id="test-int", enabled_tools=[]) + + assert "PYTHONPATH" in captured["env"], \ + "execute_code never reached Popen" + parts = captured["env"]["PYTHONPATH"].split(os.pathsep) + # Windows path comparison is case-insensitive: the inherited entries + # and the re-added repo root can carry a different case than the + # resolve()/abspath()-derived spellings used in this test (e.g. a + # launcher-written lowercase PYTHONPATH). Normalize with + # os.path.normcase so a case-only difference never fails the + # composition contract (identity on POSIX). + norm_parts = [os.path.normcase(p) for p in parts] + norm_staging = os.path.normcase(captured["staging"]) + norm_root = os.path.normcase(hermes_root) + norm_venv = os.path.normcase(venv_sp) + norm_user_a = os.path.normcase(user_a) + norm_user_b = os.path.normcase(user_b) + assert norm_parts[0] == norm_staging, \ + "staging tmpdir must be the first PYTHONPATH entry" + assert norm_venv not in norm_parts, \ + "inherited Hermes venv site-packages must be stripped" + assert norm_user_a in norm_parts and norm_user_b in norm_parts, \ + "user PYTHONPATH entries must survive" + assert norm_parts.index(norm_user_a) > norm_parts.index(norm_staging), \ + "user entries must come after the staging tmpdir" + if same_env: + assert norm_parts.count(norm_root) == 1, \ + "repo root must be re-added exactly once for a same-env child" + assert norm_parts.index(norm_user_a) > norm_parts.index(norm_root), \ + "user entries must come after the re-added repo root" + else: + assert norm_root not in norm_parts, \ + "repo root must stay absent for an external-env child" + + + def test_repo_root_direct_child_preserved(self): + """A direct child of the repo root (depth=1) is PRESERVED. + + Independent audit of every real launcher producer (Electron + ``apps/desktop/electron/main.ts``, + ``gateway/run.py::_ensure_windows_gateway_venv_imports``, + ``cron/scheduler.py::_windows_cron_python_invocation``, + ``tui_gateway/host_supervisor.py``) shows they all inject the exact + repo root and/or the venv site-packages — none injects + ``/tools`` or another direct child as an independent + PYTHONPATH entry. A user path that merely happens to live under + the repo directory must therefore be preserved. + """ + from tools.environments.local_pythonpath import _strip_hermes_owned_pythonpath + + local_file = Path(__import__("tools.environments.local", fromlist=["__file__"]).__file__).resolve() + real_repo_root = local_file.parents[2] + direct_child = str(real_repo_root / "tools") + + env = { + "PYTHONPATH": os.pathsep.join([direct_child, "/home/user/my-lib"]), + } + _strip_hermes_owned_pythonpath(env) + pp = env.get("PYTHONPATH", "") + entries = pp.split(os.pathsep) if pp else [] + assert direct_child in entries + assert "/home/user/my-lib" in entries + + def test_configured_home_alias_matches_launcher_output(self, tmp_path, monkeypatch): + """The real producer spelling is derived and consumed end to end.""" + import tools.environments.local as local + from tools.environments import local_pythonpath + from hermes_cli.gateway_windows import _preserve_hermes_home_path + + physical_home = tmp_path / "physical-home" + physical_root = _physical_repo_root(tmp_path) + configured_home = tmp_path / "configured-home" + try: + _make_directory_link(configured_home, physical_home) + except OSError as exc: + pytest.skip(f"directory link unavailable on this host: {exc}") + monkeypatch.setenv("HERMES_HOME", str(configured_home)) + + launcher_entry = Path(_preserve_hermes_home_path(physical_root)) + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home, + ) + + assert launcher_entry == configured_home / "hermes-agent" + assert launcher_entry in aliases + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + nested_user_path = launcher_entry / "user-data" + env = { + "PYTHONPATH": os.pathsep.join([ + str(launcher_entry), + str(nested_user_path), + "/home/user/my-lib", + ]) + } + local_pythonpath._strip_hermes_owned_pythonpath(env) + + assert env["PYTHONPATH"].split(os.pathsep) == [ + str(nested_user_path), + "/home/user/my-lib", + ] + + def test_profile_rehome_keeps_junction_lexical_alias(self, tmp_path, monkeypatch): + """Profile re-home must not lose the launcher's lexical repo-root spelling. + + The desktop/CLI spawn children with HERMES_HOME and PYTHONPATH in the + configured (junction) spelling, but --profile / sticky active_profile + re-home HERMES_HOME through resolve_profile_env() before the + sanitizer loads. Regression (junction + profile re-home): the alias + builder must still recover the lexical root so the inherited lexical + repo-root entry is stripped. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + from hermes_cli.profiles import resolve_profile_env + + physical_home = tmp_path / "physical-home" + physical_root = physical_home / "hermes-agent" + physical_root.mkdir(parents=True) + (physical_home / "profiles" / "coder").mkdir(parents=True) + (physical_home / "profiles" / "coder" / "config.yaml").write_text("{}\n") # identity marker + configured_home = tmp_path / "configured-home" + try: + _make_directory_link(configured_home, physical_home) + except OSError as exc: + pytest.skip(f"directory link unavailable on this host: {exc}") + + # Launcher contract: the configured spelling is the env and the root. + monkeypatch.setenv("HERMES_HOME", str(configured_home)) + lexical_root = configured_home / "hermes-agent" + + # Profile re-home keeps the configured spelling (physically identical + # through the link; lexically the launcher spelling is preserved). + assert Path(resolve_profile_env("default")) == configured_home + assert Path(resolve_profile_env("coder")) == configured_home / "profiles" / "coder" + + # The sanitizer now runs under the re-homed (profile) HERMES_HOME. + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home / "profiles" / "coder", + ) + assert any(local_pythonpath._same_path(a, lexical_root) for a in aliases) + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + env = {"PYTHONPATH": os.pathsep.join([str(lexical_root), "/home/user/my-lib"])} + local_pythonpath._strip_hermes_owned_pythonpath(env) + assert env["PYTHONPATH"].split(os.pathsep) == ["/home/user/my-lib"] + + + def test_repo_level_junction_recovers_lexical_alias(self, tmp_path, monkeypatch): + """The repo itself may be a junction under the configured root + (e.g. D:\\hermes\\hermes-agent -> C:\\...\\hermes-agent) while the + editable import spelling resolves to the physical location. The + alias builder must recover the lexical spelling via exact-identity + proof (strict resolve), not a name-based guess. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + + physical_root = _physical_repo_root(tmp_path) + configured_home = tmp_path / "configured-home" + configured_home.mkdir() + # repo-level link: /hermes-agent -> physical repo + try: + _make_directory_link(configured_home / "hermes-agent", physical_root) + except OSError as exc: + pytest.skip(f"directory link unavailable on this host: {exc}") + + lexical_root = configured_home / "hermes-agent" + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home, + ) + assert any(local_pythonpath._same_path(a, lexical_root) for a in aliases) + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + env = {"PYTHONPATH": os.pathsep.join([str(lexical_root), "/home/user/my-lib"])} + local_pythonpath._strip_hermes_owned_pythonpath(env) + assert env["PYTHONPATH"].split(os.pathsep) == ["/home/user/my-lib"] + + def test_same_named_non_owned_directories_preserved(self, tmp_path, monkeypatch): + """Negative controls: a directory that merely shares the repo's name + -- whether under the configured root or in an unrelated location -- + is never aliased or stripped. Exact filesystem identity decides, + not the name; no ownership provenance means no strip. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + + physical_root = _physical_repo_root(tmp_path) + configured_home = tmp_path / "configured-home" + (configured_home / "hermes-agent").mkdir(parents=True) + unrelated = tmp_path / "user-tools" / "hermes-agent" + unrelated.mkdir(parents=True) + + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home, + ) + for lookalike in (configured_home / "hermes-agent", unrelated): + assert not any(local_pythonpath._same_path(a, lookalike) for a in aliases) + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + for lookalike in (configured_home / "hermes-agent", unrelated): + env = {"PYTHONPATH": os.pathsep.join([str(lookalike), "/home/user/my-lib"])} + local_pythonpath._strip_hermes_owned_pythonpath(env) + assert env["PYTHONPATH"].split(os.pathsep) == [str(lookalike), "/home/user/my-lib"] + + def test_profile_home_with_repo_level_junction(self, tmp_path, monkeypatch): + """Profile re-home + repo-level junction together: the configured home + is /profiles/ while the repo is a link at /hermes-agent. + The root spelling must be derived (profiles -> grandparent) and then + the lexical repo alias recovered from it. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + + physical_root = _physical_repo_root(tmp_path) + configured_root = tmp_path / "configured-root" + (configured_root / "profiles" / "coder").mkdir(parents=True) + try: + _make_directory_link(configured_root / "hermes-agent", physical_root) + except OSError as exc: + pytest.skip(f"directory link unavailable on this host: {exc}") + + configured_home = configured_root / "profiles" / "coder" + lexical_root = configured_root / "hermes-agent" + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home, + ) + assert any(local_pythonpath._same_path(a, lexical_root) for a in aliases) + assert not any(local_pythonpath._same_path(a, configured_home / "hermes-agent") for a in aliases) + + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + env = {"PYTHONPATH": os.pathsep.join([str(lexical_root), "/home/user/my-lib"])} + local_pythonpath._strip_hermes_owned_pythonpath(env) + assert env["PYTHONPATH"].split(os.pathsep) == ["/home/user/my-lib"] + + def test_validated_runtime_venv_lexical_after_repo_recovery(self, tmp_path, monkeypatch): + """uv-base gateway: once the lexical repo alias is recovered, a lexical + VIRTUAL_ENV (/venv) validates and its site-packages is + stripped together with the repo root, while user entries survive. + """ + import tools.environments.local as local + from tools.environments import local_pythonpath + + physical_root = _physical_repo_root(tmp_path) + venv_dir = physical_root / "venv" + venv_dir.mkdir(parents=True) + (venv_dir / "pyvenv.cfg").write_text("home = x\n", encoding="utf-8") + configured_home = tmp_path / "configured-home" + configured_home.mkdir() + try: + _make_directory_link(configured_home / "hermes-agent", physical_root) + except OSError as exc: + pytest.skip(f"directory link unavailable on this host: {exc}") + + lexical_root = configured_home / "hermes-agent" + aliases = local_pythonpath._build_hermes_repo_root_aliases( + physical_root.resolve(), + physical_root, + configured_home, + ) + assert any(local_pythonpath._same_path(a, lexical_root) for a in aliases) + monkeypatch.setattr(local, "_hermes_repo_root_aliases", aliases) + + lexical_venv = lexical_root / "venv" + validated = local_pythonpath._validated_runtime_venv({"VIRTUAL_ENV": str(lexical_venv)}) + assert validated is not None + assert local_pythonpath._same_path(validated, lexical_venv) + + local._hermes_site_packages = None + env = {"PYTHONPATH": os.pathsep.join([ + str(lexical_root), + str(lexical_venv / "Lib" / "site-packages"), + "/home/user/my-lib", + ]), "VIRTUAL_ENV": str(lexical_venv)} + local_pythonpath._strip_hermes_owned_pythonpath(env) + assert env["PYTHONPATH"].split(os.pathsep) == ["/home/user/my-lib"] + + + + + +class TestPythonhomeSanitized: + """PYTHONHOME must not leak from the Hermes runtime into subprocesses. + + The gateway inherits/sets PYTHONHOME in its process environment; a child + interpreter (system Python, another venv, cron no_agent scripts) that + inherits it redirects its stdlib search to the Hermes venv and crashes + with version-mismatch errors before importing anything (#75018). + """ + + @pytest.mark.parametrize("builder", [ + "_make_run_env", + "_sanitize_subprocess_env", + "hermes_subprocess_env", + "build_subprocess_env", + ]) + def test_builders_strip_pythonhome(self, builder): + """The gateway's inherited PYTHONHOME must not reach any subprocess + builder -- terminal, background/PTY, cron no_agent scripts, and + execute_code children (#75018). + """ + from tools.environments import local as local_mod + + seed = { + "PATH": "/usr/bin:/bin", + "HOME": "/home/user", + "PYTHONHOME": "/opt/hermes-venv", + } + with patch.dict(os.environ, seed, clear=True): + if builder == "_make_run_env": + result = local_mod._make_run_env({}) + elif builder == "_sanitize_subprocess_env": + result = local_mod._sanitize_subprocess_env(dict(os.environ)) + elif builder == "hermes_subprocess_env": + result = local_mod.hermes_subprocess_env() + else: + result = local_mod.build_subprocess_env() + assert "PYTHONHOME" not in result + + def test_pythonhome_removed_from_active_venv_markers(self): + """PYTHONHOME is part of _ACTIVE_VENV_MARKER_VARS so all builders + that iterate it drop the variable.""" + from tools.environments.local_env_policy import _ACTIVE_VENV_MARKER_VARS + assert "PYTHONHOME" in _ACTIVE_VENV_MARKER_VARS + + def test_build_subprocess_env_no_scrub_preserves_pythonhome(self): + """``build_subprocess_env(scrub_secrets=False)`` is the documented + byte-for-byte escape hatch: no key is removed, so PYTHONHOME (and + everything else) survives there by contract, not by omission. + + Callers that explicitly opt out of scrubbing (git credential flows, + secret CLIs) must not have their environment silently altered — this + test pins that exception as intentional. + """ + from tools.environments.local import build_subprocess_env + base = { + "PATH": "/usr/bin:/bin", + "HOME": "/home/user", + "PYTHONHOME": "/opt/hermes-venv", + "VIRTUAL_ENV": "/opt/hermes-venv", + "SERVICE_TOKEN": "s3cr3t", + } + result = build_subprocess_env(base, scrub_secrets=False) + assert result.get("PYTHONHOME") == "/opt/hermes-venv" + assert result.get("VIRTUAL_ENV") == "/opt/hermes-venv" + assert result.get("SERVICE_TOKEN") == "s3cr3t" + + +class TestProfileScopedPassthrough: + def test_make_run_env_uses_active_profile_for_passthrough(self, monkeypatch): + """Allowlisted values must come from the routed profile, not os.environ.""" + from agent import secret_scope as ss + from tools.env_passthrough import clear_env_passthrough, register_env_passthrough + from tools.environments.local import _make_run_env + + clear_env_passthrough() + register_env_passthrough(["SERVICE_TOKEN"]) + monkeypatch.setenv("SERVICE_TOKEN", "token-for-default") + ss.set_multiplex_active(True) + token = ss.set_secret_scope({"SERVICE_TOKEN": "token-for-routed-profile"}) + try: + result = _make_run_env({}) + finally: + ss.reset_secret_scope(token) + ss.set_multiplex_active(False) + clear_env_passthrough() + + assert result["SERVICE_TOKEN"] == "token-for-routed-profile" + + def test_make_run_env_omits_missing_scoped_passthrough(self, monkeypatch): + """A missing routed secret must not fall back to the default profile.""" + from agent import secret_scope as ss + from tools.env_passthrough import clear_env_passthrough, register_env_passthrough + from tools.environments.local import _make_run_env + + clear_env_passthrough() + register_env_passthrough(["SERVICE_TOKEN"]) + monkeypatch.setenv("SERVICE_TOKEN", "token-for-default") + ss.set_multiplex_active(True) + token = ss.set_secret_scope({}) + try: + result = _make_run_env({}) + finally: + ss.reset_secret_scope(token) + ss.set_multiplex_active(False) + clear_env_passthrough() + + assert "SERVICE_TOKEN" not in result + + +class TestBlocklistCoverage: + """Sanity checks that the blocklist covers all known providers.""" + + def test_issue_1002_offenders(self): + """Blocklist includes the main offenders from issue #1002.""" + must_block = { + "OPENAI_BASE_URL", + "OPENAI_API_KEY", + "OPENROUTER_API_KEY", + "ANTHROPIC_API_KEY", + "LLM_MODEL", + } + assert must_block.issubset(_HERMES_PROVIDER_ENV_BLOCKLIST) + + def test_registry_vars_are_in_blocklist(self): + """Every api_key_env_var and base_url_env_var from PROVIDER_REGISTRY + must appear in the blocklist — ensures no drift. + + CLAUDE_CODE_OAUTH_TOKEN is the one deliberate exemption: it is owned + by the user's Claude Code install, not Hermes (#55878). + """ + from hermes_cli.auth import PROVIDER_REGISTRY + + exempt = {"CLAUDE_CODE_OAUTH_TOKEN"} + for pconfig in PROVIDER_REGISTRY.values(): + for var in pconfig.api_key_env_vars: + if var in exempt: + continue + assert var in _HERMES_PROVIDER_ENV_BLOCKLIST, ( + f"Registry var {var} (provider={pconfig.id}) missing from blocklist" + ) + if pconfig.base_url_env_var: + assert pconfig.base_url_env_var in _HERMES_PROVIDER_ENV_BLOCKLIST, ( + f"Registry base_url_env_var {pconfig.base_url_env_var} " + f"(provider={pconfig.id}) missing from blocklist" + ) + + def test_bedrock_bearer_token_is_in_blocklist(self): + """auth_type='aws_sdk' providers contribute their Hermes-managed + inference token (the Bedrock bearer) to the blocklist, keyed off + auth_type so any future SDK-cred provider is covered automatically.""" + assert "AWS_BEARER_TOKEN_BEDROCK" in _HERMES_PROVIDER_ENV_BLOCKLIST + + def test_general_aws_chain_not_in_blocklist(self): + """The general AWS credential chain must NOT be in the blocklist — + no-regression guard for #32314. These belong to the user's trusted + operator shell (SECURITY.md §3.2), not to Hermes, and blocklisting + them would be unrecoverable via env_passthrough (GHSA-rhgp-j443-p4rf). + """ + general_chain = { + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "AWS_PROFILE", + "AWS_DEFAULT_REGION", + "AWS_REGION", + "AWS_SHARED_CREDENTIALS_FILE", + "AWS_CONFIG_FILE", + "AWS_WEB_IDENTITY_TOKEN_FILE", + "AWS_ROLE_ARN", + } + leaked_block = general_chain & _HERMES_PROVIDER_ENV_BLOCKLIST + assert not leaked_block, ( + f"General AWS chain vars must stay inheritable, but these are " + f"blocklisted: {sorted(leaked_block)} (capability regression, #32314)" + ) + + def test_extra_auth_vars_covered(self): + """Non-registry auth vars (ANTHROPIC_TOKEN) must also be in the + blocklist.""" + extras = {"ANTHROPIC_TOKEN"} + assert extras.issubset(_HERMES_PROVIDER_ENV_BLOCKLIST) + + def test_claude_code_oauth_token_is_inheritable(self): + """CLAUDE_CODE_OAUTH_TOKEN is owned by the user's Claude Code install + (subscription OAuth), not a Hermes inference credential. Stripping it + made agent-spawned ``claude`` fall through to the shared Keychain / + ~/.claude credential store and clobber the user's interactive login + on auth failure (#55878). It must stay inheritable.""" + assert "CLAUDE_CODE_OAUTH_TOKEN" not in _HERMES_PROVIDER_ENV_BLOCKLIST + + def test_non_registry_provider_vars_are_in_blocklist(self): + extras = { + "GOOGLE_API_KEY", + "DEEPSEEK_API_KEY", + "MISTRAL_API_KEY", + "GROQ_API_KEY", + "TOGETHER_API_KEY", + "PERPLEXITY_API_KEY", + "COHERE_API_KEY", + "FIREWORKS_API_KEY", + "XAI_API_KEY", + "HELICONE_API_KEY", + } + assert extras.issubset(_HERMES_PROVIDER_ENV_BLOCKLIST) + + def test_optional_tool_and_messaging_vars_are_in_blocklist(self): + """Tool/messaging vars from OPTIONAL_ENV_VARS should stay covered.""" + from hermes_cli.config import OPTIONAL_ENV_VARS + + for name, metadata in OPTIONAL_ENV_VARS.items(): + category = metadata.get("category") + if category in {"tool", "messaging"}: + assert name in _HERMES_PROVIDER_ENV_BLOCKLIST, ( + f"Optional env var {name} (category={category}) missing from blocklist" + ) + elif category == "setting" and metadata.get("password"): + assert name in _HERMES_PROVIDER_ENV_BLOCKLIST, ( + f"Secret setting env var {name} missing from blocklist" + ) + + def test_gateway_runtime_vars_are_in_blocklist(self): + extras = { + "TELEGRAM_HOME_CHANNEL", + "TELEGRAM_HOME_CHANNEL_NAME", + "DISCORD_HOME_CHANNEL", + "DISCORD_HOME_CHANNEL_NAME", + "DISCORD_REQUIRE_MENTION", + "DISCORD_FREE_RESPONSE_CHANNELS", + "DISCORD_AUTO_THREAD", + "SLACK_HOME_CHANNEL", + "SLACK_HOME_CHANNEL_NAME", + "SLACK_ALLOWED_USERS", + "WHATSAPP_ENABLED", + "WHATSAPP_MODE", + "WHATSAPP_ALLOWED_USERS", + "SIGNAL_HTTP_URL", + "SIGNAL_ACCOUNT", + "SIGNAL_ALLOWED_USERS", + "SIGNAL_GROUP_ALLOWED_USERS", + "SIGNAL_HOME_CHANNEL", + "SIGNAL_HOME_CHANNEL_NAME", + "SIGNAL_IGNORE_STORIES", + "HASS_TOKEN", + "HASS_URL", + "EMAIL_ADDRESS", + "EMAIL_PASSWORD", + "EMAIL_IMAP_HOST", + "EMAIL_SMTP_HOST", + "EMAIL_HOME_ADDRESS", + "EMAIL_HOME_ADDRESS_NAME", + "HERMES_DASHBOARD_SESSION_TOKEN", + "GATEWAY_ALLOWED_USERS", + "GH_TOKEN", + "GITHUB_APP_ID", + "GITHUB_APP_PRIVATE_KEY_PATH", + "GITHUB_APP_INSTALLATION_ID", + "MODAL_TOKEN_ID", + "MODAL_TOKEN_SECRET", + "DAYTONA_API_KEY", + "VERCEL_OIDC_TOKEN", + "VERCEL_TOKEN", + "VERCEL_PROJECT_ID", + "VERCEL_TEAM_ID", + } + assert extras.issubset(_HERMES_PROVIDER_ENV_BLOCKLIST) + + +class TestSanePathIncludesHomebrew: + """Verify _SANE_PATH includes macOS Homebrew directories.""" + + @pytest.fixture(autouse=True) + def _disable_hermes_bin_injection(self): + """These tests assert the sane-path merge in isolation. Disable the + hermes-install-dir prepend (a separate concern, covered by + TestHermesBinDirOnPath) so a real ``hermes`` on the test runner's PATH + doesn't shift the asserted PATH layout.""" + from tools.environments import local as local_mod + saved = local_mod._HERMES_BIN_DIR + local_mod._HERMES_BIN_DIR = None # resolved -> no dir to inject + yield + local_mod._HERMES_BIN_DIR = saved + + def test_sane_path_includes_homebrew_bin(self): + from tools.environments.local import _SANE_PATH + assert "/opt/homebrew/bin" in _SANE_PATH + + + def test_make_run_env_appends_homebrew_on_minimal_path(self, monkeypatch): + """When PATH is minimal, _make_run_env appends missing sane entries. + + POSIX: the sane-path merge appends the Homebrew dirs. Windows: + _append_missing_sane_path_entries is a documented passthrough (the + native PATH must not be touched), so the assertion is the unchanged + input. Git Bash dir prepending is neutralised so the merged PATH + layout is deterministic on every host. + """ + from tools.environments import local as local_mod + from tools.environments.local import _SANE_PATH, _make_run_env + monkeypatch.setattr(local_mod, "_git_bash_bin_dirs", lambda: []) + minimal_env = {"PATH": "/some/custom/bin"} + with patch.dict(os.environ, minimal_env, clear=True): + result = _make_run_env({}) + path_entries = result["PATH"].split(os.pathsep) + assert path_entries[0] == "/some/custom/bin" + if sys.platform == "win32": + assert result["PATH"] == "/some/custom/bin" + else: + for entry in _SANE_PATH.split(os.pathsep): + assert entry in path_entries + + @pytest.mark.platforms("macos") def test_make_run_env_real_launchd_path_gains_homebrew(self): """The literal macOS launchd PATH is the production trigger for #35613. 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 5d093354de..f610fa574b 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 @@ -616,6 +614,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 {})) @@ -763,6 +762,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: @@ -863,50 +864,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, @@ -961,11 +919,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 @@ -1008,9 +966,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": { @@ -1018,15 +976,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", @@ -1121,7 +1079,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/desktop.md b/website/docs/user-guide/desktop.md index 87db35cdbe..9cd79eb9a6 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. diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index 6ab7aa9f7b..2341db4c3f 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