From 5c4ebaaefe64e123145c2bd517ebba477b73a7eb Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 12:31:03 -0700 Subject: [PATCH 01/10] feat(hooks): sync user-hooks/ to host on general config save --- .../sync_instance_configs_and_restart.yml | 20 +++++++++++++ tests/test_instance_api_routes.py | 30 +++++++++++++++++++ tests/test_sync_configs_playbook_hooks.py | 28 +++++++++++++++++ ui/routes/instance_routes.py | 1 + 4 files changed, 79 insertions(+) create mode 100644 tests/test_sync_configs_playbook_hooks.py diff --git a/ansible/playbooks/sync_instance_configs_and_restart.yml b/ansible/playbooks/sync_instance_configs_and_restart.yml index 59795a1..520966c 100644 --- a/ansible/playbooks/sync_instance_configs_and_restart.yml +++ b/ansible/playbooks/sync_instance_configs_and_restart.yml @@ -58,6 +58,26 @@ delegate_to: localhost become: false + - name: Ensure user-hooks directory exists on host + ansible.builtin.file: + path: "{{ qlds_dir }}/user-hooks" + state: directory + owner: ql + group: ql + mode: '0755' + + - name: Sync user LD_PRELOAD hooks from UI server to instance user-hooks dir + ansible.builtin.synchronize: + src: "../../configs/{{ host_name }}/{{ qlds_id }}/user-hooks/" + dest: "{{ qlds_dir }}/user-hooks/" + delete: yes + rsync_path: "sudo -u ql rsync" + rsync_opts: + - "--include=*.so" + - "--exclude=*" + delegate_to: localhost + become: false + - name: Backfill common minqlx-plugins to instance directory ansible.builtin.command: argv: diff --git a/tests/test_instance_api_routes.py b/tests/test_instance_api_routes.py index fd324b2..c1e2991 100644 --- a/tests/test_instance_api_routes.py +++ b/tests/test_instance_api_routes.py @@ -598,6 +598,36 @@ def test_update_config_enabled_hooks_without_draft_filters_to_existing_files( assert updated.ld_preload_hooks == 'existing_hook.so' +def test_update_config_creates_user_hooks_source_dir_when_absent( + client, app, auth_token, sample_instance, tmp_path, monkeypatch +): + """ + GIVEN an instance whose local configs///user-hooks/ dir does not + exist (e.g. deployed before the hook feature) + WHEN PUT /api/instances//config is called with enabled_hooks + THEN the request succeeds AND the user-hooks/ source dir is created, so the + general-save rsync source is never missing (spec §5.4). + """ + monkeypatch.chdir(tmp_path) + instance, host = sample_instance + + hooks_dir = tmp_path / 'configs' / host.name / str(instance.id) / 'user-hooks' + assert not hooks_dir.exists() + + payload = {'configs': _full_configs(), 'enabled_hooks': [], 'restart': False} + + with patch('ui.routes.instance_routes.acquire_lock', return_value=True), \ + patch('ui.routes.instance_routes.enqueue_task', return_value=MagicMock(id='job-1')): + response = client.put( + f'/api/instances/{instance.id}/config', + json=payload, + headers=_auth_header(auth_token), + ) + + assert response.status_code == 202, response.get_json() + assert hooks_dir.is_dir() + + @pytest.mark.parametrize( ('configs', 'message'), [ diff --git a/tests/test_sync_configs_playbook_hooks.py b/tests/test_sync_configs_playbook_hooks.py new file mode 100644 index 0000000..1943381 --- /dev/null +++ b/tests/test_sync_configs_playbook_hooks.py @@ -0,0 +1,28 @@ +import yaml + +PLAYBOOK = "ansible/playbooks/sync_instance_configs_and_restart.yml" +SYNC_KEY = "ansible.builtin.synchronize" + + +def _tasks(): + with open(PLAYBOOK) as f: + doc = yaml.safe_load(f) + return doc[0]["tasks"] + + +def test_playbook_ensures_user_hooks_dir(): + names = [t.get("name", "") for t in _tasks()] + assert any("user-hooks directory exists" in n for n in names), names + + +def test_playbook_syncs_user_hooks_with_delete(): + sync = next( + t for t in _tasks() + if SYNC_KEY in t and "user-hooks/" in t[SYNC_KEY].get("src", "") + ) + s = sync[SYNC_KEY] + assert s["dest"] == "{{ qlds_dir }}/user-hooks/" + assert s["delete"] is True + assert s["src"] == "../../configs/{{ host_name }}/{{ qlds_id }}/user-hooks/" + assert "--include=*.so" in s["rsync_opts"] + assert "--exclude=*" in s["rsync_opts"] diff --git a/ui/routes/instance_routes.py b/ui/routes/instance_routes.py index 3d49f3d..76a5764 100644 --- a/ui/routes/instance_routes.py +++ b/ui/routes/instance_routes.py @@ -1014,6 +1014,7 @@ def manage_instance_config_api(instance_id): # Renamed and combined GET/POST fro # Handle scripts via draft or legacy instance_user_hooks_dir = os.path.join(instance_config_dir, 'user-hooks') + os.makedirs(instance_user_hooks_dir, exist_ok=True) # rsync source must exist (spec §5.4) if draft_id: from ui.routes.draft_routes import ( _get_draft_base_path, _get_draft_scripts_path, _get_draft_user_hooks_path, From 127ebd4dd2b925faf829a88b77c1799180e98743 Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 12:47:20 -0700 Subject: [PATCH 02/10] feat(hooks): fold hook enable/order into Save Configuration --- .../instances/EditInstanceConfigModal.jsx | 153 ++++++++++++---- .../src/components/instances/HooksTab.jsx | 173 ++++-------------- .../EditInstanceConfigModal.test.jsx | 111 +++++++++-- .../instances/__tests__/HooksTab.test.jsx | 143 ++++++--------- .../src/services/__tests__/api.test.js | 2 + frontend-react/src/services/api.js | 4 + 6 files changed, 318 insertions(+), 268 deletions(-) diff --git a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx index a418295..3a72184 100644 --- a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx +++ b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useRef, useCallback } from 'react'; +import React, { useState, useEffect, useRef, useCallback, useMemo } from 'react'; import { Dialog, DialogBackdrop } from '@headlessui/react'; import { X, LoaderCircle, Zap, AlertTriangle, Settings, Code2, LayoutGrid, Save, FolderOpen, RotateCw, Webhook } from 'lucide-react'; import { json, jsonParseLinter } from '@codemirror/lang-json'; @@ -130,10 +130,19 @@ function EditInstanceConfigModal({ const [initialCheckedPlugins, setInitialCheckedPlugins] = useState(new Set()); const [scriptHostName, setScriptHostName] = useState(null); const [draftPreset, setDraftPreset] = useState(null); // null = seed from instance; string = seed from preset - const [presetEnabledHooks, setPresetEnabledHooks] = useState(null); // null = no preset loaded this session const [rawQlxPlugins, setRawQlxPlugins] = useState([]); // bare plugin names from instance const pluginFileManagerRef = useRef(null); const pluginsSyncedRef = useRef(false); + const hookLoadSeqRef = useRef(0); + + // Hooks tab state + const [hookAvailable, setHookAvailable] = useState([]); + const [hookMissing, setHookMissing] = useState([]); + const [hookSystem, setHookSystem] = useState([]); + const [hookEnabledOrder, setHookEnabledOrder] = useState([]); + const [initialHookEnabledOrder, setInitialHookEnabledOrder] = useState([]); + const [hooksLoaded, setHooksLoaded] = useState(false); + const [instanceStatus, setInstanceStatus] = useState(null); // Factories tab state const [factoryServerTree, setFactoryServerTree] = useState([]); @@ -249,9 +258,41 @@ function EditInstanceConfigModal({ [pluginDraftId, instanceId], ); + const loadHooks = useCallback(async ({ seedInitial } = {}) => { + const loadSeq = hookLoadSeqRef.current + 1; + hookLoadSeqRef.current = loadSeq; + try { + const data = await fetchInstanceHooks(instanceId); + if (hookLoadSeqRef.current !== loadSeq) return; + const all = data.available || []; + const available = all.filter((hook) => !hook.missing); + const missing = all.filter((hook) => hook.missing).map((hook) => hook.filename); + const present = new Set(available.map((hook) => hook.filename)); + + setHookAvailable(available); + setHookMissing(missing); + setHookSystem(data.system_hooks_active || []); + if (seedInitial) { + const initOrder = available + .filter((hook) => hook.enabled) + .sort((a, b) => a.order - b.order) + .map((hook) => hook.filename); + setHookEnabledOrder(initOrder); + setInitialHookEnabledOrder(initOrder); + } else { + setHookEnabledOrder((prev) => prev.filter((filename) => present.has(filename))); + } + setHooksLoaded(true); + } catch (err) { + if (hookLoadSeqRef.current !== loadSeq) return; + if (seedInitial) setHooksLoaded(false); + console.error('EditInstanceConfigModal: Hooks fetch error:', err); + } + }, [instanceId]); useEffect(() => { if (isOpen && instanceId) { + let cancelled = false; setCurrentInstanceName(initialInstanceName || `Instance ${instanceId}`); const fetchInitialData = async () => { setLoading(true); @@ -264,7 +305,13 @@ function EditInstanceConfigModal({ setActiveMainTab(initialTab); setScriptHostName(null); setDraftPreset(null); - setPresetEnabledHooks(null); + setHookAvailable([]); + setHookMissing([]); + setHookSystem([]); + setHookEnabledOrder([]); + setInitialHookEnabledOrder([]); + setHooksLoaded(false); + setInstanceStatus(null); pluginsSyncedRef.current = false; setFactoryServerTree([]); @@ -277,6 +324,7 @@ function EditInstanceConfigModal({ getInstanceConfig(instanceId), getPresets(), ]); + if (cancelled) return; // Store raw plugin names — they'll be resolved to full tree paths // once the draft tree loads (via the effect below). @@ -295,6 +343,7 @@ function EditInstanceConfigModal({ setScriptHostName(fetchedHostName); setHostOsType(instanceDetails.host_os_type || null); setHostLanRateUsesHook(instanceDetails.host_lan_rate_uses_hook === true); + setInstanceStatus(instanceDetails.status || null); const incomingFolders = Array.isArray(configData?.config_folders) ? configData.config_folders : []; @@ -327,17 +376,27 @@ function EditInstanceConfigModal({ setOriginalLanRateEnabled(instanceDetails.lan_rate_enabled || false); setIsDirty(false); // Reset dirty state setPresets(presetsData || []); + await loadHooks({ seedInitial: true }); } catch (err) { - setError(err.message || `Failed to fetch initial data for instance ${instanceId}`); - console.error("EditInstanceConfigModal: Initial data fetch error:", err); + if (!cancelled) { + setError(err.message || `Failed to fetch initial data for instance ${instanceId}`); + console.error("EditInstanceConfigModal: Initial data fetch error:", err); + } } finally { - setLoading(false); - setLoadingPresets(false); + if (!cancelled) { + setLoading(false); + setLoadingPresets(false); + } } }; fetchInitialData(); + return () => { + cancelled = true; + hookLoadSeqRef.current += 1; + }; } - }, [isOpen, instanceId, initialInstanceName, initialTab, resetConfigs, resetFactories]); + return undefined; + }, [isOpen, instanceId, initialInstanceName, initialTab, resetConfigs, resetFactories, loadHooks]); // Sync effect: Update serverHostname when server.cfg changes (unless it's an internal update) useEffect(() => { @@ -396,6 +455,11 @@ function EditInstanceConfigModal({ }; const checkedPluginsChanged = !setsEqual(checkedPlugins, initialCheckedPlugins); + const hooksDirty = useMemo( + () => hookEnabledOrder.length !== initialHookEnabledOrder.length + || hookEnabledOrder.some((filename, index) => initialHookEnabledOrder[index] !== filename), + [hookEnabledOrder, initialHookEnabledOrder], + ); const metadataChanged = currentInstanceName !== originalInstanceName || serverHostname !== originalServerHostname || @@ -410,12 +474,14 @@ function EditInstanceConfigModal({ factoriesHaveChanges || pluginsHaveChanges || checkedPluginsChanged || + hooksDirty || metadataChanged )); }, [ checkedPluginsChanged, configsHaveChanges, factoriesHaveChanges, + hooksDirty, isOpen, loading, metadataChanged, @@ -444,7 +510,10 @@ function EditInstanceConfigModal({ resetFactories(factoriesToLoad); setCheckedPlugins(new Set(presetData.checked_plugins || [])); setDraftPreset(presetData.name); - setPresetEnabledHooks(presetData.enabled_hooks ?? null); + if (presetData.enabled_hooks !== undefined && presetData.enabled_hooks !== null) { + setHookEnabledOrder(presetData.enabled_hooks); + setHooksLoaded(true); + } // Refresh the factory tree to show the preset's available factories try { @@ -615,8 +684,8 @@ function EditInstanceConfigModal({ .filter(p => p.endsWith('.py') && !p.endsWith('__init__.py')) .map(p => p.replace(/\.py$/, '').replace(/^.*\//, '')), }; - if (presetEnabledHooks !== null) { - configPayload.enabled_hooks = presetEnabledHooks; + if (hooksLoaded) { + configPayload.enabled_hooks = hookEnabledOrder; } // Pass restart parameter to updateInstanceConfig @@ -646,12 +715,20 @@ function EditInstanceConfigModal({ } }; - const handleHooksApplied = () => { - showSuccess('LD_PRELOAD hooks saved. Apply task queued.'); - onConfigSaved?.(); - setIsDirty(false); - onClose(); - }; + const handleToggleHook = useCallback((filename) => { + setHookEnabledOrder((cur) => ( + cur.includes(filename) ? cur.filter((name) => name !== filename) : [...cur, filename] + )); + }, []); + + const handleReorderHooks = useCallback((nextOrder) => setHookEnabledOrder(nextOrder), []); + + const handleRemoveMissingHook = useCallback((filename) => { + setHookMissing((cur) => cur.filter((name) => name !== filename)); + setHookEnabledOrder((cur) => cur.filter((name) => name !== filename)); + }, []); + + const handleRefreshHooks = useCallback(() => loadHooks(), [loadHooks]); const handleExpandEditor = (selectedFile, content = '') => { const fileNameToExpand = typeof selectedFile === 'string' @@ -968,7 +1045,19 @@ function EditInstanceConfigModal({ {activeMainTab === 'hooks' && (
- +
)} @@ -998,7 +1087,7 @@ function EditInstanceConfigModal({ )} - {/* Right side - Esc hint + Cancel + Save (Save hidden on hooks tab) */} + {/* Right side - Esc hint + Cancel + Save */}
Esc @@ -1007,20 +1096,18 @@ function EditInstanceConfigModal({ - {activeMainTab !== 'hooks' && ( - - )} +
diff --git a/frontend-react/src/components/instances/HooksTab.jsx b/frontend-react/src/components/instances/HooksTab.jsx index b103fb9..c8b97fe 100644 --- a/frontend-react/src/components/instances/HooksTab.jsx +++ b/frontend-react/src/components/instances/HooksTab.jsx @@ -1,8 +1,8 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useMemo, useRef, useState } from 'react'; import { flushSync } from 'react-dom'; import { closestCenter, DndContext } from '@dnd-kit/core'; import { arrayMove, SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'; -import { deleteInstanceHook, fetchInstanceHooks, saveInstanceHooks, uploadInstanceHook } from '../../services/api'; +import { deleteInstanceHook, uploadInstanceHook } from '../../services/api'; import HookRow, { MissingHookRow, ReadOnlyHookRow, SortableHookRow } from './HookRow'; import ConfirmationModal from '../ConfirmationModal'; @@ -10,69 +10,23 @@ function errorMessage(error, fallback) { return error?.error?.message || error?.message || fallback; } -export default function HooksTab({ instanceId, draftId, onApplied }) { +export default function HooksTab({ + instanceId, + available = [], + missing = [], + systemHooks = [], + enabledOrder = [], + dirty = false, + onToggleHook, + onReorderHooks, + onRemoveMissing, + onRefresh, +}) { const uploadRef = useRef(null); const scrollRef = useRef(null); - const [reloadKey, setReloadKey] = useState(0); - const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const [available, setAvailable] = useState([]); - const [enabledOrder, setEnabledOrder] = useState([]); - const [initialEnabled, setInitialEnabled] = useState([]); - const [systemHooks, setSystemHooks] = useState([]); - const [applying, setApplying] = useState(false); const [uploading, setUploading] = useState(false); - const [missingHooks, setMissingHooks] = useState([]); - const [initialMissing, setInitialMissing] = useState([]); const [pendingDelete, setPendingDelete] = useState(null); - const [hasReplacedHook, setHasReplacedHook] = useState(false); - - const reload = () => setReloadKey((k) => k + 1); - - // Reset to blank loading state only when the target instance/draft changes. - useEffect(() => { - setLoading(true); - setError(null); - setAvailable([]); - setEnabledOrder([]); - setInitialEnabled([]); - setSystemHooks([]); - setMissingHooks([]); - setInitialMissing([]); - setHasReplacedHook(false); - }, [instanceId, draftId]); - - // Fetch (or silently re-fetch on reload). Does NOT set loading=true so - // uploads/deletes refresh without blanking the tab. - useEffect(() => { - let cancelled = false; - setError(null); - fetchInstanceHooks(instanceId, draftId) - .then((data) => { - if (cancelled) return; - const all = data.available || []; - const enabled = all - .filter((hook) => hook.enabled && !hook.missing) - .sort((a, b) => a.order - b.order) - .map((hook) => hook.filename); - const missing = all.filter((hook) => hook.missing).map((hook) => hook.filename); - setAvailable(all.filter((hook) => !hook.missing)); - setEnabledOrder(enabled); - setInitialEnabled(enabled); - setMissingHooks(missing); - setInitialMissing(missing); - setSystemHooks(data.system_hooks_active || []); - }) - .catch((err) => { - if (!cancelled) setError(errorMessage(err, 'Failed to load hooks.')); - }) - .finally(() => { - if (!cancelled) setLoading(false); - }); - return () => { - cancelled = true; - }; - }, [instanceId, draftId, reloadKey]); const enabledSet = useMemo(() => new Set(enabledOrder), [enabledOrder]); const enabledRows = useMemo( @@ -89,21 +43,15 @@ export default function HooksTab({ instanceId, draftId, onApplied }) { .sort((a, b) => a.filename.localeCompare(b.filename)), [available, enabledSet], ); - const removeMissingHook = (filename) => { - setMissingHooks((current) => current.filter((f) => f !== filename)); - }; - const handleUpload = async (e) => { const file = e.target.files?.[0]; e.target.value = ''; if (!file) return; - const isReplacement = available.some((h) => h.filename === file.name); setUploading(true); setError(null); try { await uploadInstanceHook(instanceId, file); - if (isReplacement) setHasReplacedHook(true); - reload(); + onRefresh?.(); } catch (err) { setError(errorMessage(err, 'Upload failed.')); } finally { @@ -114,20 +62,13 @@ export default function HooksTab({ instanceId, draftId, onApplied }) { const confirmDelete = async () => { try { await deleteInstanceHook(instanceId, pendingDelete.filename); - reload(); + setPendingDelete(null); + onRefresh?.(); } catch (err) { setError(errorMessage(err, 'Delete failed.')); } }; - const dirty = useMemo( - () => hasReplacedHook - || enabledOrder.length !== initialEnabled.length - || enabledOrder.some((filename, index) => initialEnabled[index] !== filename) - || missingHooks.length !== initialMissing.length, - [hasReplacedHook, enabledOrder, initialEnabled, missingHooks, initialMissing], - ); - const restrictToTab = useCallback(({ transform, draggingNodeRect }) => { if (!scrollRef.current || !draggingNodeRect) return { ...transform, x: 0 }; const bounds = scrollRef.current.getBoundingClientRect(); @@ -137,11 +78,7 @@ export default function HooksTab({ instanceId, draftId, onApplied }) { }, []); const toggleHook = (filename) => { - const doUpdate = () => setEnabledOrder((current) => ( - current.includes(filename) - ? current.filter((item) => item !== filename) - : [...current, filename] - )); + const doUpdate = () => onToggleHook?.(filename); if (document.startViewTransition) { document.startViewTransition(() => flushSync(doUpdate)); } else { @@ -151,41 +88,12 @@ export default function HooksTab({ instanceId, draftId, onApplied }) { const handleDragEnd = ({ active, over }) => { if (!over || active.id === over.id) return; - setEnabledOrder((current) => { - const oldIndex = current.indexOf(active.id); - const newIndex = current.indexOf(over.id); - if (oldIndex === -1 || newIndex === -1) return current; - return arrayMove(current, oldIndex, newIndex); - }); - }; - - const handleCancel = () => { - setEnabledOrder(initialEnabled); - setMissingHooks(initialMissing); - setHasReplacedHook(false); - setError(null); + const oldIndex = enabledOrder.indexOf(active.id); + const newIndex = enabledOrder.indexOf(over.id); + if (oldIndex === -1 || newIndex === -1) return; + onReorderHooks?.(arrayMove(enabledOrder, oldIndex, newIndex)); }; - const handleApply = async () => { - setApplying(true); - setError(null); - try { - await saveInstanceHooks(instanceId, enabledOrder, draftId); - setInitialEnabled(enabledOrder); - setInitialMissing(missingHooks); - setHasReplacedHook(false); - onApplied?.(); - } catch (err) { - setError(errorMessage(err, 'Failed to save hooks.')); - } finally { - setApplying(false); - } - }; - - if (loading) { - return
Loading hooks...
; - } - return (
{error && ( @@ -193,6 +101,11 @@ export default function HooksTab({ instanceId, draftId, onApplied }) { {error}
)} + {dirty && ( +
+ Unsaved hook changes — click Save Configuration to apply. +
+ )} {systemHooks.length > 0 && (
@@ -208,9 +121,9 @@ export default function HooksTab({ instanceId, draftId, onApplied }) {
User hooks - {!draftId && instanceId && ( + {instanceId && ( <> - +
-
- - -
setPendingDelete(null)} diff --git a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx index 4f7282e..aa94cc8 100644 --- a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx +++ b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx @@ -6,6 +6,7 @@ const mocks = vi.hoisted(() => ({ createPreset: vi.fn(), downloadPreset: vi.fn(), flushEdits: vi.fn(), + fetchInstanceHooks: vi.fn(), getBinaryMeta: vi.fn(), getFactoryContent: vi.fn(), getFactoryTree: vi.fn(), @@ -49,6 +50,7 @@ vi.mock('../../../services/api', () => ({ downloadPreset: mocks.downloadPreset, getFactoryContent: mocks.getFactoryContent, getFactoryTree: mocks.getFactoryTree, + fetchInstanceHooks: mocks.fetchInstanceHooks, getInstanceById: mocks.getInstanceById, getInstanceConfig: mocks.getInstanceConfig, getPresetById: mocks.getPresetById, @@ -121,8 +123,8 @@ vi.mock('../HooksTab', () => ({ return (
hooks-tab
-
); @@ -212,13 +214,22 @@ describe('EditInstanceConfigModal preset saving', () => { mocks.createPreset.mockResolvedValue({ message: 'saved', data: { id: 42, name: 'saved-from-edit' } }); mocks.downloadPreset.mockResolvedValue(new Blob(['zip-bytes'], { type: 'application/zip' })); mocks.flushEdits.mockResolvedValue(undefined); + mocks.fetchInstanceHooks.mockResolvedValue({ + available: [ + { filename: 'a.so', size: 1, modified: 1, enabled: true, order: 1, description: '' }, + { filename: 'c.so', size: 1, modified: 1, enabled: false, order: null, description: '' }, + ], + missing: [], + system_hooks_active: [], + }); mocks.getBinaryMeta.mockResolvedValue({}); mocks.getFactoryContent.mockResolvedValue({ content: '' }); mocks.getFactoryTree.mockResolvedValue([]); mocks.getInstanceById.mockResolvedValue({ host_name: 'test-host', lan_rate_enabled: false, - name: 'Test123', + status: 'running', + name: 'inst', qlx_plugins: 'balance', }); mocks.getInstanceConfig.mockResolvedValue({ @@ -232,7 +243,7 @@ describe('EditInstanceConfigModal preset saving', () => { mocks.getPresets.mockResolvedValue([]); mocks.saveBinaryMeta.mockResolvedValue({}); mocks.updateInstance.mockResolvedValue({}); - mocks.updateInstanceConfig.mockResolvedValue({ message: 'saved' }); + mocks.updateInstanceConfig.mockResolvedValue({ message: 'ok' }); mocks.updatePreset.mockResolvedValue({ message: 'updated', data: { id: 42, name: 'saved-from-edit' } }); mocks.useDraftWorkspace.mockReturnValue({ draftId: 'draft-123', @@ -385,6 +396,82 @@ describe('EditInstanceConfigModal preset saving', () => { ); }); + it('shows the Save Configuration button on the Hooks tab', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: /save configuration/i })).toBeInTheDocument(); + }); + + it('Save payload carries enabled_hooks reflecting toggles', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); + fireEvent.click(screen.getByRole('button', { name: /mock toggle c.so/i })); + fireEvent.click(screen.getByRole('button', { name: /save configuration/i })); + + await waitFor(() => expect(mocks.updateInstanceConfig).toHaveBeenCalledTimes(1)); + expect(mocks.updateInstanceConfig.mock.calls[0][1].enabled_hooks).toEqual(['a.so', 'c.so']); + }); + + it('does NOT send enabled_hooks when the hooks fetch failed on open', async () => { + mocks.fetchInstanceHooks.mockRejectedValueOnce(new Error('hooks unavailable')); + + render( + + ); + + await waitFor(() => expect(screen.getByRole('button', { name: /save configuration/i })).toBeInTheDocument()); + fireEvent.click(screen.getByRole('button', { name: /save configuration/i })); + + await waitFor(() => expect(mocks.updateInstanceConfig).toHaveBeenCalledTimes(1)); + expect(mocks.updateInstanceConfig.mock.calls[0][1]).not.toHaveProperty('enabled_hooks'); + }); + + it('an unrelated Save preserves the loaded hooks', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByRole('button', { name: /save configuration/i })).toBeInTheDocument()); + fireEvent.click(screen.getByRole('button', { name: /save configuration/i })); + + await waitFor(() => expect(mocks.updateInstanceConfig).toHaveBeenCalledTimes(1)); + expect(mocks.updateInstanceConfig.mock.calls[0][1].enabled_hooks).toEqual(['a.so']); + }); + it('disables enabling 99k lan rate for ubuntu hosts', async () => { mocks.getInstanceById.mockResolvedValue({ host_name: 'ubuntu-host', @@ -575,7 +662,7 @@ describe('EditInstanceConfigModal preset saving', () => { expect(factoryManagerProps.onExpandEditor).toEqual(expect.any(Function)); }); - it('opens on the hooks tab and closes after hook apply', async () => { + it('opens on the hooks tab with controlled hook props', async () => { const onClose = vi.fn(); const onConfigSaved = vi.fn(); @@ -592,12 +679,12 @@ describe('EditInstanceConfigModal preset saving', () => { await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); expect(screen.getByRole('button', { name: /^hooks$/i })).toHaveClass('text-[var(--accent-primary)]'); - - fireEvent.click(screen.getByRole('button', { name: /mock apply hooks/i })); - - expect(mocks.showSuccess).toHaveBeenCalledWith('LD_PRELOAD hooks saved. Apply task queued.'); - expect(onConfigSaved).toHaveBeenCalled(); - expect(onClose).toHaveBeenCalled(); - expect(mocks.hooksTabProps.at(-1).instanceId).toBe(1); + expect(onConfigSaved).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + expect(mocks.hooksTabProps.at(-1)).toEqual(expect.objectContaining({ + instanceId: 1, + enabledOrder: ['a.so'], + dirty: false, + })); }); }); diff --git a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx index ee10a26..c4b61d1 100644 --- a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx +++ b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx @@ -5,23 +5,33 @@ import * as api from '../../../services/api'; vi.mock('../../../services/api'); -function hooksResponse(overrides = {}) { - return { - available: [ - { filename: 'a.so', size: 1024, modified: 1, enabled: true, order: 1, description: 'Speed hook' }, - { filename: 'b.so', size: 2048, modified: 1, enabled: true, order: 2, description: '' }, - { filename: 'c.so', size: 3072, modified: 1, enabled: false, order: null, description: '' }, - ], - system_hooks_active: [], +const baseAvailable = [ + { filename: 'a.so', size: 1024, modified: 1, enabled: true, order: 1, description: 'Speed hook' }, + { filename: 'b.so', size: 2048, modified: 1, enabled: true, order: 2, description: '' }, + { filename: 'c.so', size: 3072, modified: 1, enabled: false, order: null, description: '' }, +]; + +function renderTab(overrides = {}) { + const props = { + instanceId: 1, + available: baseAvailable, + missing: [], + systemHooks: [], + enabledOrder: ['a.so', 'b.so'], + dirty: false, + onToggleHook: vi.fn(), + onReorderHooks: vi.fn(), + onRemoveMissing: vi.fn(), + onRefresh: vi.fn(), ...overrides, }; + const view = render(); + return { ...view, props }; } describe('HooksTab', () => { beforeEach(() => { vi.clearAllMocks(); - api.fetchInstanceHooks.mockResolvedValue(hooksResponse()); - api.saveInstanceHooks.mockResolvedValue({ task_id: 't1' }); api.uploadInstanceHook.mockResolvedValue({}); api.deleteInstanceHook.mockResolvedValue({}); api.renameInstanceHook.mockResolvedValue({}); @@ -29,129 +39,95 @@ describe('HooksTab', () => { api.downloadInstanceHook.mockResolvedValue(new Blob()); }); - it('renders user hooks with enabled state and order', async () => { - render(); + it('renders user hooks with enabled state and order', () => { + renderTab(); - await waitFor(() => expect(screen.getByTestId('hook-row-a.so')).toBeInTheDocument()); + expect(screen.getByTestId('hook-row-a.so')).toBeInTheDocument(); expect(screen.getByRole('button', { name: /enable a.so/i })).toHaveAttribute('aria-pressed', 'true'); expect(screen.getByRole('button', { name: /enable c.so/i })).toHaveAttribute('aria-pressed', 'false'); }); - it('hides system hooks section when system_hooks_active is empty', async () => { - render(); + it('hides system hooks section when systemHooks is empty', () => { + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); expect(screen.queryByText(/system hooks/i)).not.toBeInTheDocument(); }); - it('shows system hooks section when non-empty', async () => { - api.fetchInstanceHooks.mockResolvedValueOnce(hooksResponse({ - system_hooks_active: [{ filename: 'force_rate.so', size: 15880 }], - })); + it('shows system hooks section when non-empty', () => { + renderTab({ systemHooks: [{ filename: 'force_rate.so', size: 15880 }] }); - render(); - - await waitFor(() => expect(screen.getByText(/system hooks/i)).toBeInTheDocument()); + expect(screen.getByText(/system hooks/i)).toBeInTheDocument(); expect(screen.getByText('force_rate')).toBeInTheDocument(); }); - it('toggling a hook marks the form dirty and enables Apply', async () => { - render(); + it('has no in-tab Apply/Cancel footer', () => { + renderTab({ dirty: true }); - await waitFor(() => screen.getByTestId('hook-row-a.so')); - const apply = screen.getByRole('button', { name: /apply.*restart/i }); - expect(apply).toBeDisabled(); - fireEvent.click(screen.getByRole('button', { name: /enable c.so/i })); - expect(apply).toBeEnabled(); + expect(screen.queryByRole('button', { name: /apply.*restart/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /^cancel$/i })).not.toBeInTheDocument(); }); - it('Apply sends the enabled list in display order', async () => { - render(); + it('toggling c.so calls onToggleHook', async () => { + const { props } = renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); fireEvent.click(screen.getByRole('button', { name: /enable c.so/i })); - fireEvent.click(screen.getByRole('button', { name: /apply.*restart/i })); - await waitFor(() => expect(api.saveInstanceHooks).toHaveBeenCalled()); - expect(api.saveInstanceHooks).toHaveBeenCalledWith(1, ['a.so', 'b.so', 'c.so'], undefined); + await waitFor(() => expect(props.onToggleHook).toHaveBeenCalledWith('c.so')); }); - it('renders hook descriptions from GET data', async () => { - render(); + it('renders hook descriptions from controlled data', () => { + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); expect(screen.getByText('Speed hook')).toBeInTheDocument(); }); - it('calls onApplied after a successful save', async () => { - const onApplied = vi.fn(); - render(); - - await waitFor(() => screen.getByTestId('hook-row-a.so')); - fireEvent.click(screen.getByRole('button', { name: /enable c.so/i })); - fireEvent.click(screen.getByRole('button', { name: /apply.*restart/i })); - - await waitFor(() => expect(onApplied).toHaveBeenCalled()); - }); - - it('shows Upload .so button for live instance', async () => { - render(); + it('shows Upload .so button for live instance', () => { + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); expect(screen.getByRole('button', { name: /upload .so/i })).toBeInTheDocument(); }); - it('hides Upload .so button when draftId is set', async () => { - render(); - - await waitFor(() => screen.getByTestId('hook-row-a.so')); - expect(screen.queryByRole('button', { name: /upload .so/i })).not.toBeInTheDocument(); - }); - - it('upload calls uploadInstanceHook and reloads the list', async () => { - render(); - - await waitFor(() => screen.getByTestId('hook-row-a.so')); - expect(api.fetchInstanceHooks).toHaveBeenCalledTimes(1); + it('upload calls uploadInstanceHook and onRefresh after success', async () => { + const { props } = renderTab(); const file = new File(['ELF content'], 'new.so', { type: 'application/octet-stream' }); - const input = document.querySelector('input[type="file"][accept=".so"]'); - fireEvent.change(input, { target: { files: [file] } }); + fireEvent.change(screen.getByTestId('hook-upload-input'), { target: { files: [file] } }); await waitFor(() => expect(api.uploadInstanceHook).toHaveBeenCalledWith(1, file)); - await waitFor(() => expect(api.fetchInstanceHooks).toHaveBeenCalledTimes(2)); + expect(props.onRefresh).toHaveBeenCalledTimes(1); }); it('shows error banner when upload fails', async () => { api.uploadInstanceHook.mockRejectedValueOnce({ error: { message: 'Not an ELF file' } }); - render(); + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); const file = new File(['bad'], 'bad.so', { type: 'application/octet-stream' }); - const input = document.querySelector('input[type="file"][accept=".so"]'); - fireEvent.change(input, { target: { files: [file] } }); + fireEvent.change(screen.getByTestId('hook-upload-input'), { target: { files: [file] } }); await waitFor(() => expect(screen.getByText('Not an ELF file')).toBeInTheDocument()); }); + it('dirty hint appears when dirty', () => { + renderTab({ dirty: true }); + + expect(screen.getByText(/Unsaved hook changes/i)).toBeInTheDocument(); + expect(screen.getByText(/click Save Configuration to apply/i)).toBeInTheDocument(); + }); + it('delete via actions menu shows confirmation modal', async () => { - render(); + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); const row = screen.getByTestId('hook-row-a.so'); fireEvent.click(within(row).getByRole('button', { name: /actions for a.so/i })); fireEvent.click(within(row).getByRole('menuitem', { name: /delete/i, hidden: true })); const dialog = await screen.findByRole('dialog'); expect(within(dialog).getByText(/delete hook\?/i)).toBeInTheDocument(); - expect(within(dialog).getByText(/will be permanently deleted/i)).toBeInTheDocument(); + expect(within(dialog).getByText(/This cannot be undone/i)).toBeInTheDocument(); }); - it('confirming delete calls deleteInstanceHook and reloads', async () => { - render(); - - await waitFor(() => screen.getByTestId('hook-row-a.so')); - expect(api.fetchInstanceHooks).toHaveBeenCalledTimes(1); + it('confirming delete calls deleteInstanceHook and onRefresh', async () => { + const { props } = renderTab(); const row = screen.getByTestId('hook-row-a.so'); fireEvent.click(within(row).getByRole('button', { name: /actions for a.so/i })); @@ -161,13 +137,12 @@ describe('HooksTab', () => { fireEvent.click(within(dialog).getByRole('button', { name: /delete/i })); await waitFor(() => expect(api.deleteInstanceHook).toHaveBeenCalledWith(1, 'a.so')); - await waitFor(() => expect(api.fetchInstanceHooks).toHaveBeenCalledTimes(2)); + expect(props.onRefresh).toHaveBeenCalledTimes(1); }); it('cancelling delete dismisses the modal', async () => { - render(); + renderTab(); - await waitFor(() => screen.getByTestId('hook-row-a.so')); const row = screen.getByTestId('hook-row-a.so'); fireEvent.click(within(row).getByRole('button', { name: /actions for a.so/i })); fireEvent.click(within(row).getByRole('menuitem', { name: /delete/i, hidden: true })); @@ -175,7 +150,7 @@ describe('HooksTab', () => { const dialog = await screen.findByRole('dialog'); fireEvent.click(within(dialog).getByRole('button', { name: /cancel/i })); - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(api.deleteInstanceHook).not.toHaveBeenCalled(); }); }); diff --git a/frontend-react/src/services/__tests__/api.test.js b/frontend-react/src/services/__tests__/api.test.js index 1baa88d..cf11bac 100644 --- a/frontend-react/src/services/__tests__/api.test.js +++ b/frontend-react/src/services/__tests__/api.test.js @@ -103,6 +103,7 @@ describe('updateInstanceConfig', () => { draft_id: 'draft-456', checked_plugins: ['balance.py'], lan_rate_enabled: true, + enabled_hooks: ['a.so', 'b.so'], }, true, ); @@ -118,6 +119,7 @@ describe('updateInstanceConfig', () => { draft_id: 'draft-456', checked_plugins: ['balance.py'], lan_rate_enabled: true, + enabled_hooks: ['a.so', 'b.so'], factories: { 'base.factories': 'factory contents' }, }); }); diff --git a/frontend-react/src/services/api.js b/frontend-react/src/services/api.js index b877f9d..6d270c4 100644 --- a/frontend-react/src/services/api.js +++ b/frontend-react/src/services/api.js @@ -314,6 +314,7 @@ export const updateInstanceConfig = async (instanceId, configData, restart = tru draft_id, checked_plugins, lan_rate_enabled, + enabled_hooks, name, hostname, ...configs @@ -346,6 +347,9 @@ export const updateInstanceConfig = async (instanceId, configData, restart = tru if (lan_rate_enabled !== undefined) { payload.lan_rate_enabled = lan_rate_enabled; } + if (enabled_hooks !== undefined) { + payload.enabled_hooks = enabled_hooks; + } const response = await apiClient.put(`/instances/${instanceId}/config`, payload); return response.data; // Assuming API returns { "message": "..." } } catch (error) { From 0d20e2a4f5d96ab6b3792d2c25c12a39523c27e3 Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 13:00:18 -0700 Subject: [PATCH 03/10] feat(hooks): force restart on hook change for running instances --- .../instances/EditInstanceConfigModal.jsx | 25 +++++++----- .../EditInstanceConfigModal.test.jsx | 38 +++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx index 3a72184..26ecccf 100644 --- a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx +++ b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx @@ -449,17 +449,24 @@ function EditInstanceConfigModal({ setIsDirty(true); }; - const handleRestartToggle = () => { - if (lanRateChanged) return; - setRestartAfterSave(prev => !prev); - }; - const checkedPluginsChanged = !setsEqual(checkedPlugins, initialCheckedPlugins); const hooksDirty = useMemo( () => hookEnabledOrder.length !== initialHookEnabledOrder.length || hookEnabledOrder.some((filename, index) => initialHookEnabledOrder[index] !== filename), [hookEnabledOrder, initialHookEnabledOrder], ); + const hooksForceRestart = hooksDirty && instanceStatus === 'running'; + const restartForced = lanRateChanged || hooksForceRestart; + + useEffect(() => { + if (hooksForceRestart) setRestartAfterSave(true); + }, [hooksForceRestart]); + + const handleRestartToggle = () => { + if (restartForced) return; + setRestartAfterSave(prev => !prev); + }; + const metadataChanged = currentInstanceName !== originalInstanceName || serverHostname !== originalServerHostname || @@ -944,7 +951,7 @@ function EditInstanceConfigModal({ - + Restart after saving - {lanRateChanged && ( - + {restartForced && ( + )}
diff --git a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx index aa94cc8..bd7681b 100644 --- a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx +++ b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx @@ -432,6 +432,44 @@ describe('EditInstanceConfigModal preset saving', () => { expect(mocks.updateInstanceConfig.mock.calls[0][1].enabled_hooks).toEqual(['a.so', 'c.so']); }); + it('forces + disables restart toggle when hooks change on a RUNNING instance', async () => { + mocks.getInstanceById.mockResolvedValue({ host_name: 'h', lan_rate_enabled: false, status: 'running', name: 'i' }); + + render( + + ); + + await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); + fireEvent.click(screen.getByText('Mock Toggle c.so')); + const toggle = screen.getByRole('button', { name: /toggle restart after save/i }); + await waitFor(() => expect(toggle).toBeDisabled()); + expect(toggle).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getByText('Changing hooks requires an instance restart')).toBeInTheDocument(); + }); + + it('does NOT force restart when hooks change on a STOPPED instance', async () => { + mocks.getInstanceById.mockResolvedValue({ host_name: 'h', lan_rate_enabled: false, status: 'stopped', name: 'i' }); + + render( + + ); + + await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); + fireEvent.click(screen.getByText('Mock Toggle c.so')); + const toggle = screen.getByRole('button', { name: /toggle restart after save/i }); + expect(toggle).not.toBeDisabled(); + }); + it('does NOT send enabled_hooks when the hooks fetch failed on open', async () => { mocks.fetchInstanceHooks.mockRejectedValueOnce(new Error('hooks unavailable')); From 90b9e4b0517e3bcac688044cb0004638e281887e Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 13:11:21 -0700 Subject: [PATCH 04/10] refactor(hooks): remove orphaned PUT /hooks route and client call --- .../src/services/__tests__/api.test.js | 8 -- frontend-react/src/services/api.js | 15 --- tests/test_instance_hooks_routes.py | 125 +----------------- ui/routes/instance_hooks_routes.py | 99 +------------- 4 files changed, 7 insertions(+), 240 deletions(-) diff --git a/frontend-react/src/services/__tests__/api.test.js b/frontend-react/src/services/__tests__/api.test.js index cf11bac..2a00582 100644 --- a/frontend-react/src/services/__tests__/api.test.js +++ b/frontend-react/src/services/__tests__/api.test.js @@ -28,7 +28,6 @@ import { getSelfHostDefaults, importPreset, resizeHost, - saveInstanceHooks, updateInstanceConfig, } from '../api'; @@ -155,13 +154,6 @@ describe('instance hook API helpers', () => { }); expect(mocks.get).toHaveBeenCalledWith('/instances/12/hooks'); }); - - it('saveInstanceHooks uses the shared api client for PUT', async () => { - mocks.put.mockResolvedValueOnce({ data: { data: { task_id: 'job-1' } } }); - - await expect(saveInstanceHooks(12, ['a.so'])).resolves.toEqual({ task_id: 'job-1' }); - expect(mocks.put).toHaveBeenCalledWith('/instances/12/hooks', { enabled: ['a.so'] }); - }); }); describe('importPreset', () => { diff --git a/frontend-react/src/services/api.js b/frontend-react/src/services/api.js index 6d270c4..6718539 100644 --- a/frontend-react/src/services/api.js +++ b/frontend-react/src/services/api.js @@ -381,21 +381,6 @@ export const fetchInstanceHooks = async (instanceId, draftId) => { } }; -export const saveInstanceHooks = async (instanceId, enabledFilenames, draftId) => { - try { - const body = { enabled: enabledFilenames }; - if (draftId) body.draft_id = draftId; - const response = await apiClient.put( - `/instances/${instanceId}/hooks`, - body, - ); - return response.data.data; - } catch (error) { - console.error(`Failed to save hooks for instance ${instanceId}:`, error.response ? error.response.data : error.message); - throw error.response ? error.response.data : new Error(`Failed to save hooks for instance ${instanceId}`); - } -}; - export const uploadInstanceHook = async (instanceId, file) => { const form = new FormData(); form.append('file', file); diff --git a/tests/test_instance_hooks_routes.py b/tests/test_instance_hooks_routes.py index c91b218..1672bbf 100644 --- a/tests/test_instance_hooks_routes.py +++ b/tests/test_instance_hooks_routes.py @@ -1,7 +1,4 @@ -import json import os -from types import SimpleNamespace -from unittest.mock import patch import pytest @@ -49,14 +46,6 @@ def instance_with_scripts(app, tmp_path, monkeypatch): yield inst -def _put(client, headers, instance_id, body): - return client.put( - f"/api/instances/{instance_id}/hooks", - data=json.dumps(body), - headers={**headers, "Content-Type": "application/json"}, - ) - - def test_get_lists_available_with_enabled_and_order(client, headers, instance_with_scripts): response = client.get(f"/api/instances/{instance_with_scripts.id}/hooks", headers=headers) assert response.status_code == 200 @@ -147,113 +136,11 @@ def test_get_requires_auth(client, instance_with_scripts): assert response.status_code == 401 -def test_put_rejects_missing_body(client, headers, instance_with_scripts): - response = client.put( - f"/api/instances/{instance_with_scripts.id}/hooks", - data="not json", - headers={**headers, "Content-Type": "application/json"}, - ) - assert response.status_code == 400 - - -def test_put_rejects_non_list(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": "a.so"}) - assert response.status_code == 400 - - -def test_put_rejects_path_traversal(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["../etc/passwd.so"]}) - assert response.status_code == 400 - - -def test_put_rejects_non_so_extension(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["evil.txt"]}) - assert response.status_code == 400 - - -def test_put_rejects_reserved_filename(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["force_rate.so"]}) - assert response.status_code == 400 - assert "reserved" in response.get_json()["error"]["message"].lower() - - -def test_put_rejects_nonexistent_file(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["nope.so"]}) - assert response.status_code == 400 - - -def test_put_rejects_non_elf(client, headers, instance_with_scripts, tmp_path): - bad = tmp_path / "configs" / "h1" / str(instance_with_scripts.id) / "user-hooks" / "bad.so" - bad.write_bytes(b"NOT_ELF") - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["bad.so"]}) - assert response.status_code == 400 - - -def test_put_rejects_duplicates(client, headers, instance_with_scripts): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["a.so", "a.so"]}) - assert response.status_code == 400 +def test_put_hooks_route_removed(client): + resp = client.put('/api/instances/1/hooks', json={'enabled': []}) + assert resp.status_code == 405 -def test_put_returns_409_when_instance_lock_held(client, headers, instance_with_scripts): - with patch("ui.routes.instance_hooks_routes.acquire_lock", return_value=False): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["a.so"]}) - assert response.status_code == 409 - - -def test_put_enqueue_failure_reverts_hooks_and_marks_error( - app, - client, - headers, - instance_with_scripts, -): - original = instance_with_scripts.ld_preload_hooks - with patch("ui.routes.instance_hooks_routes.acquire_lock", return_value=True), \ - patch("ui.routes.instance_hooks_routes.release_lock") as release, \ - patch("ui.routes.instance_hooks_routes.enqueue_apply_hooks", return_value=None): - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["b.so"]}) - - assert response.status_code == 500 - with app.app_context(): - fresh = db.session.get(QLInstance, instance_with_scripts.id) - assert fresh.ld_preload_hooks == original - assert fresh.status == InstanceStatus.ERROR - release.assert_called_once() - - -def test_put_stopped_instance_enqueues_without_restart(app, client, headers, instance_with_scripts): - with app.app_context(): - instance_with_scripts.status = InstanceStatus.STOPPED - db.session.commit() - - with patch("ui.routes.instance_hooks_routes.acquire_lock", return_value=True), \ - patch("ui.routes.instance_hooks_routes.release_lock"), \ - patch("ui.routes.instance_hooks_routes.enqueue_apply_hooks") as enq: - enq.return_value = SimpleNamespace(id="job-id-stopped") - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["a.so"]}) - - assert response.status_code == 202 - assert enq.call_args.kwargs["restart_service"] is False - - -def test_put_rejects_file_present_only_in_legacy_scripts(client, headers, instance_with_scripts, tmp_path): - scripts = tmp_path / "configs" / "h1" / str(instance_with_scripts.id) / "scripts" - scripts.mkdir(parents=True) - (scripts / "legacy.so").write_bytes(b"\x7fELF") - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["legacy.so"]}) - assert response.status_code == 400 - assert "not found" in response.get_json()["error"]["message"].lower() - - -def test_put_happy_path_persists_and_enqueues(app, client, headers, instance_with_scripts): - with patch("ui.routes.instance_hooks_routes.acquire_lock", return_value=True), \ - patch("ui.routes.instance_hooks_routes.release_lock"), \ - patch("ui.routes.instance_hooks_routes.enqueue_apply_hooks") as enq: - enq.return_value = SimpleNamespace(id="job-id-123") - response = _put(client, headers, instance_with_scripts.id, {"enabled": ["b.so", "a.so"]}) - - assert response.status_code == 202 - assert response.get_json()["data"]["task_id"] == "job-id-123" - with app.app_context(): - fresh = db.session.get(QLInstance, instance_with_scripts.id) - assert fresh.ld_preload_hooks == "b.so,a.so" - assert fresh.status == InstanceStatus.CONFIGURING +def test_apply_instance_hooks_task_still_importable(): + from ui.tasks import apply_instance_hooks + assert callable(apply_instance_hooks) diff --git a/ui/routes/instance_hooks_routes.py b/ui/routes/instance_hooks_routes.py index 94705ef..f650d05 100644 --- a/ui/routes/instance_hooks_routes.py +++ b/ui/routes/instance_hooks_routes.py @@ -1,14 +1,11 @@ import os import re -import shutil -import uuid from flask import Blueprint, jsonify, request from flask_jwt_extended import jwt_required from ui import db -from ui.models import BinaryMetadata, InstanceStatus, QLInstance -from ui.task_lock import acquire_lock, release_lock +from ui.models import BinaryMetadata, QLInstance from ui.task_logic.ansible_instance_mgmt import RESERVED_HOOK_FILENAMES, _SYSTEM_HOOKS from ui.task_logic.ansible_instance_hooks import _system_hook_source_path from ui.task_logic.hook_paths import user_hooks_dir as _user_hooks_dir, draft_user_hooks_dir as _draft_user_hooks_dir @@ -78,19 +75,6 @@ def _is_elf_file(path): return False -def enqueue_apply_hooks(instance_id, *, restart_service, lock_token): - from ui.task_logic.job_failure_handlers import instance_job_failure_handler - from ui.tasks import apply_instance_hooks, enqueue_task - - return enqueue_task( - apply_instance_hooks, - instance_id, - restart_service=restart_service, - lock_token=lock_token, - on_failure=instance_job_failure_handler, - ) - - @instance_hooks_bp.route("//hooks", methods=["GET"]) @jwt_required() def get_instance_hooks(instance_id): @@ -150,84 +134,3 @@ def get_instance_hooks(instance_id): "system_hooks_active": system_hooks_active, }, }), 200 - - -@instance_hooks_bp.route("//hooks", methods=["PUT"]) -@jwt_required() -def put_instance_hooks(instance_id): - instance = db.session.get(QLInstance, instance_id) - if not instance: - return jsonify({"error": {"message": "Instance not found"}}), 404 - - payload = request.get_json(silent=True) - if not isinstance(payload, dict) or "enabled" not in payload: - return jsonify({"error": {"message": 'Body must be JSON {"enabled": [...]}'}}), 400 - - enabled = payload["enabled"] - if not isinstance(enabled, list) or not all(isinstance(item, str) for item in enabled): - return jsonify({"error": {"message": "enabled must be a list of strings"}}), 400 - if len(enabled) != len(set(enabled)): - return jsonify({"error": {"message": "duplicate filenames in enabled list"}}), 400 - - for name in enabled: - error = _validate_filename(name) - if error: - return jsonify({"error": {"message": f"{name}: {error}"}}), 400 - - hooks_dir = _user_hooks_dir(instance) - draft_id = payload.get('draft_id', '') or '' - use_draft = bool(draft_id and _DRAFT_ID_RE.match(draft_id)) - draft_dir = _draft_user_hooks_dir(draft_id) if use_draft else None - - for name in enabled: - full_path = os.path.join(hooks_dir, name) - if not os.path.isfile(full_path): - if draft_dir: - draft_path = os.path.join(draft_dir, name) - if os.path.isfile(draft_path): - os.makedirs(hooks_dir, exist_ok=True) - shutil.copy2(draft_path, full_path) - else: - return jsonify({"error": {"message": f"{name}: file not found in user-hooks/"}}), 400 - else: - return jsonify({"error": {"message": f"{name}: file not found in user-hooks/"}}), 400 - if not _is_elf_file(full_path): - return jsonify({"error": {"message": f"{name}: not a valid ELF binary"}}), 400 - - lock_token = str(uuid.uuid4()) - if not acquire_lock("instance", instance.id, lock_token, ttl=360): - msg = f'Another operation is running on instance "{instance.name}". Please wait for it to complete.' - return jsonify({"error": {"message": msg}}), 409 - - original_hooks = instance.ld_preload_hooks - restart_service = instance.status != InstanceStatus.STOPPED - lock_transferred = False - instance.ld_preload_hooks = ",".join(enabled) if enabled else None - instance.status = InstanceStatus.CONFIGURING - db.session.commit() - - try: - job = enqueue_apply_hooks( - instance.id, - restart_service=restart_service, - lock_token=lock_token, - ) - if job: - lock_transferred = True - return jsonify({"data": {"task_id": job.id}}), 202 - - instance.ld_preload_hooks = original_hooks - instance.status = InstanceStatus.ERROR - db.session.commit() - return jsonify({"error": {"message": "Error queuing hook apply task."}}), 500 - except Exception: - db.session.rollback() - current = db.session.get(QLInstance, instance_id) - if current: - current.ld_preload_hooks = original_hooks - current.status = InstanceStatus.ERROR - db.session.commit() - return jsonify({"error": {"message": "Error queuing hook apply task."}}), 500 - finally: - if not lock_transferred: - release_lock("instance", instance_id, lock_token) From b2371dcdac2c88e5907c45e5be095e60c101cbad Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 13:29:26 -0700 Subject: [PATCH 05/10] docs(hooks): document unified save flow and bump version --- VERSION | 2 +- docs/technical.md | 41 ++++++++++++++++++++++--------------- docs/user/features/hooks.md | 18 +++++++++++++++- docs/user/releases.md | 1 + docs/user/version.json | 2 +- 5 files changed, 44 insertions(+), 20 deletions(-) diff --git a/VERSION b/VERSION index 2e3a551..b0f139e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.13.6 +1.13.7 diff --git a/docs/technical.md b/docs/technical.md index 51d9d7a..35575fb 100644 --- a/docs/technical.md +++ b/docs/technical.md @@ -100,7 +100,7 @@ The Flask application follows the application factory pattern, which provides se - `index_routes.py`: Handles the main index page. * `host_routes.py`: Handles CRUD operations for Hosts, protected by `@jwt_required()`. * `instance_routes.py`: Handles CRUD operations for QLInstances, protected by `@jwt_required()`. - * `instance_hooks_routes.py`: Lists and updates per-instance LD_PRELOAD hook selections from uploaded `.so` files. + * `instance_hooks_routes.py`: Lists and manages per-instance LD_PRELOAD hook files from uploaded `.so` files. Hook selection writes use `PUT /instances//config` rather than a separate user-facing apply route. * `preset_api_routes.py`: Handles CRUD operations for ConfigPresets, protected by `@jwt_required()`. Preset writes accept generic config and factory maps, plugin draft IDs, checked plugin lists, and checked factory lists. Mutating operations (rename, content update, delete) are rejected with `403` for any preset where `is_builtin = True`. * `server_status_routes.py`: Handles live status retrieval (`GET /api/server-status`) and workshop preview lookup (`GET /api/server-status/workshop-preview/`). * `settings_routes.py`: Handles application settings management (API keys, rate limit config). @@ -238,7 +238,8 @@ The project uses pytest for testing, with fixtures defined in `tests/conftest.py - **Playbook Structure:** - **`ansible/playbooks/setup_host.yml`:** Performs the initial one-time setup on a newly provisioned host. Installs prerequisites (including `iptables-persistent`, and `redis-server` only when the host runtime needs its own Redis), configures the firewall using a template (`ansible/templates/iptables.rules.j2`) that defines both filter and NAT rules which are applied atomically via `iptables-restore` and persisted, creates the `ql` user, installs base SteamCMD/QLDS/minqlx to shared locations (`/home/ql/qlds-base`, `/home/ql/minqlx-shared`), and syncs common assets (`/home/ql/assets/common`). - **`ansible/playbooks/add_qlds_instance.yml`:** Adds a new QLDS instance to a pre-configured host. Creates the instance directory (`/home/ql/qlds-{id}`), copies shared resources (QLDS base, minqlx, common assets) into the instance directory, syncs instance-specific configuration files from the UI server (`configs///`), installs instance-specific Python dependencies, and manages the systemd service. - - **`ansible/playbooks/update_instance_hooks.yml`:** Re-renders a QLDS instance systemd unit with the current LD_PRELOAD value and daemon-reloads systemd. It restarts the service only when the accepted hook update came from a non-stopped instance. + - **`ansible/playbooks/sync_instance_configs_and_restart.yml`:** Applies saved configuration changes to an existing instance. It syncs configs, factories, plugin drafts, and `user-hooks/` to the host, re-renders the service unit with the current `LD_PRELOAD`, and restarts only when requested/required. + - **`ansible/playbooks/update_instance_hooks.yml`:** System-hook maintenance path used by backend tasks to re-render a QLDS instance systemd unit with the current LD_PRELOAD value and daemon-reload systemd when a system hook changes outside the user Save Configuration flow. - **`ansible/playbooks/manage_qlds_service.yml`:** Manages the `qlds@.service` systemd service (start, stop, restart, delete service file). - **`ansible/playbooks/get_qlds_logs.yml`:** Retrieves logs for a specific instance service. - **`ansible/playbooks/setup_qlfilter.yml`:** Installs QLFilter (eBPF/XDP packet filter) on a target host. @@ -265,23 +266,29 @@ The project uses pytest for testing, with fixtures defined in `tests/conftest.py ### LD_PRELOAD Hooks -Per-instance LD_PRELOAD hooks are stored on `QLInstance.ld_preload_hooks` as a -comma-separated list of uploaded `.so` filenames. `_build_ld_preload_paths()` in -`ui/task_logic/ansible_instance_mgmt.py` converts that list into a colon-joined -path string, prepending any active system hooks. The system hook registry is -empty today; `force_rate.so` is reserved so a future built-in hook cannot be -shadowed by a user upload. +Per-instance user hook selections are stored on `QLInstance.ld_preload_hooks` as +a comma-separated list of uploaded `.so` filenames. In the user-facing save flow, +the Hooks tab sends those selections as `enabled_hooks` through +`PUT /instances//config`; the normal **Save Configuration** path validates the +list, drops entries whose binaries are not present in the instance `user-hooks/` +directory, persists the filtered order, and queues the config sync/restart task. +The dedicated user-facing `PUT /instances//hooks` route/client apply path has +been removed; hook file CRUD routes remain for upload, download, replace, rename, +delete, and description edits. + +`_build_ld_preload_paths()` in `ui/task_logic/ansible_instance_mgmt.py` converts +the stored user hook list into a colon-joined path string, prepending any active +system hooks. The system-hook task path remains available for backend-managed +system hooks such as `force_rate.so` when their predicates change outside the +user Save Configuration flow. The `qlds@.service.j2` template emits `Environment=LD_PRELOAD=...` only when -the computed value is non-empty. Existing deploy, restart, config apply, and -LAN-rate reconfigure flows pass the same `ld_preload_paths` extra-var so later -unit re-renders preserve hook state. - -The `apply_instance_hooks` RQ task delegates to -`ui/task_logic/ansible_instance_hooks.py`. It re-validates enabled hooks with a -pre-flight existence and ELF-magic check, preserves CPU affinity while -re-rendering the unit, and leaves stopped instances stopped by passing -`restart_service=false` to `update_instance_hooks.yml`. +the computed value is non-empty. Deploy, restart, LAN-rate reconfigure, and Save +Configuration flows pass the same `ld_preload_paths` extra-var so later unit +re-renders preserve hook state. `sync_instance_configs_and_restart.yml` now syncs +`user-hooks/` to the game host as part of Save Configuration before templating the +unit; running instances with hook changes force a restart, while stopped instances +are templated and left stopped. - **Terraform Run Logging:** * Detailed stdout and stderr from Terraform CLI executions (triggered by tasks in `ui/task_logic/terraform_provision.py` and `ui/task_logic/terraform_destroy.py`) are no longer stored directly in the `Host.logs` database field. * Instead, these verbose logs are saved to individual files within the `logs/terraform_runs/` directory (e.g., `logs/terraform_runs/host_____.log`). This is managed by the `save_terraform_run_log` function in `ui/task_logic/file_logger.py`. diff --git a/docs/user/features/hooks.md b/docs/user/features/hooks.md index 09a8ea7..1cd9615 100644 --- a/docs/user/features/hooks.md +++ b/docs/user/features/hooks.md @@ -32,22 +32,38 @@ QLSM validates that uploaded files are ELF binaries (checks the magic bytes). No After uploading, the hook appears in the list in disabled state. +Uploads are stored immediately on the QLSM server. The new binary is copied to the game host the next time you click **Save Configuration**. + ### Enable Or Disable A Hook -Toggle the switch on the hook row. The change takes effect on the next instance restart (or immediately if "Restart after saving" is enabled). +Toggle the switch on the hook row, then click **Save Configuration**. Hook selections are saved with the rest of the instance configuration. ### Reorder Hooks Drag hook rows to change the load order. Hooks are passed to `LD_PRELOAD` in top-to-bottom order. System hooks always load before user hooks regardless of position. +Reordering is also saved through **Save Configuration**. + ### Delete A Hook Click the delete (trash) icon on the hook row and confirm in the modal. Deleting a hook removes it from the host on the next sync. +Deletes happen immediately on the QLSM server. The removed binary and any resulting `LD_PRELOAD` changes are reflected on the game host the next time you click **Save Configuration**. + +## Saving Hook Changes + +Hook enable/disable and reorder changes use the same **Save Configuration** button as config, plugin, factory, and basic instance settings. + +- **Running instances:** hook changes require the QLDS process to restart so `LD_PRELOAD` can be rebuilt. QLSM forces the restart toggle on and disables it for that save; clicking **Save Configuration** syncs files, templates the systemd unit, and restarts the instance automatically. +- **Stopped instances:** clicking **Save Configuration** syncs files and templates the systemd unit, but the instance stays stopped. Start it manually when you are ready to run with the new hook set. +- **File uploads/deletes:** upload and delete operations happen immediately on the QLSM server, but hook binaries are copied to or removed from the game host only on the next **Save Configuration**. + ## Missing Hooks If a hook file exists in the database but its binary is missing from the host filesystem, QLSM shows a warning row with the filename highlighted. Click the remove button on that row to delete the stale entry. This can happen if host files were deleted out-of-band. +If a missing hook remains selected, QLSM drops it on the next **Save Configuration**. Stale `LD_PRELOAD` entries can block the server from starting, so missing binaries are not preserved indefinitely. + ## Load Order When an instance starts, QLSM builds the `LD_PRELOAD` value in this order: diff --git a/docs/user/releases.md b/docs/user/releases.md index 08c54e5..883461c 100644 --- a/docs/user/releases.md +++ b/docs/user/releases.md @@ -4,6 +4,7 @@ QLSM uses `v..` tags. Every merged pull request is listed a | Version | Date | PR | Changes | | --- | --- | --- | --- | +| `v1.13.7` | 2026-07-05 | — | Hooks now save through Save Configuration; removed the separate Apply & Restart button. | | `v1.13.6` | 2026-07-03 | [#136](https://github.com/dngrtech/qlsm/pull/136) | Fix presets losing their LD_PRELOAD hook files when loaded onto an instance, and make presets remember which hooks were enabled. | | `v1.13.5` | 2026-07-03 | [#135](https://github.com/dngrtech/qlsm/pull/135) | Fix preset ZIP import rejecting `.so` plugin scripts, and surface `.so` scripts correctly in the preset API instead of silently dropping them. | | `v1.13.4` | 2026-07-02 | — | Bug fixes and improvements. | diff --git a/docs/user/version.json b/docs/user/version.json index a1f077c..3f5f6f4 100644 --- a/docs/user/version.json +++ b/docs/user/version.json @@ -1,4 +1,4 @@ { - "latest": "1.13.6", + "latest": "1.13.7", "releaseNotesUrl": "https://dngrtech.github.io/qlsm/releases/" } From 94fcd8e65102244b9041f6477943d4334cef5e2f Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 13:37:05 -0700 Subject: [PATCH 06/10] fix(hooks): keep stopped instances stopped on hook save --- docs/technical.md | 67 +++++++------------ docs/user/features/hooks.md | 2 +- .../instances/EditInstanceConfigModal.jsx | 16 ++++- .../EditInstanceConfigModal.test.jsx | 8 ++- tests/test_instance_hooks_routes.py | 5 +- 5 files changed, 44 insertions(+), 54 deletions(-) diff --git a/docs/technical.md b/docs/technical.md index 35575fb..22570fc 100644 --- a/docs/technical.md +++ b/docs/technical.md @@ -269,7 +269,7 @@ The project uses pytest for testing, with fixtures defined in `tests/conftest.py Per-instance user hook selections are stored on `QLInstance.ld_preload_hooks` as a comma-separated list of uploaded `.so` filenames. In the user-facing save flow, the Hooks tab sends those selections as `enabled_hooks` through -`PUT /instances//config`; the normal **Save Configuration** path validates the +`PUT /api/instances//config`; the normal **Save Configuration** path validates the list, drops entries whose binaries are not present in the instance `user-hooks/` directory, persists the filtered order, and queues the config sync/restart task. The dedicated user-facing `PUT /instances//hooks` route/client apply path has @@ -289,10 +289,14 @@ re-renders preserve hook state. `sync_instance_configs_and_restart.yml` now sync `user-hooks/` to the game host as part of Save Configuration before templating the unit; running instances with hook changes force a restart, while stopped instances are templated and left stopped. -- **Terraform Run Logging:** - * Detailed stdout and stderr from Terraform CLI executions (triggered by tasks in `ui/task_logic/terraform_provision.py` and `ui/task_logic/terraform_destroy.py`) are no longer stored directly in the `Host.logs` database field. - * Instead, these verbose logs are saved to individual files within the `logs/terraform_runs/` directory (e.g., `logs/terraform_runs/host_____.log`). This is managed by the `save_terraform_run_log` function in `ui/task_logic/file_logger.py`. - * The `Host.logs` database field now stores concise, timestamped status messages, including a reference to the path of the detailed log file for each Terraform command executed. + +### Terraform Run Logging + +Detailed stdout and stderr from Terraform CLI executions (triggered by tasks in `ui/task_logic/terraform_provision.py` and `ui/task_logic/terraform_destroy.py`) are no longer stored directly in the `Host.logs` database field. + +Instead, these verbose logs are saved to individual files within the `logs/terraform_runs/` directory (e.g., `logs/terraform_runs/host_____.log`). This is managed by the `save_terraform_run_log` function in `ui/task_logic/file_logger.py`. + +The `Host.logs` database field now stores concise, timestamped status messages, including a reference to the path of the detailed log file for each Terraform command executed. ### QLDS CPU Affinity @@ -359,18 +363,23 @@ re-run later. ## 99k LAN Rate Mode -The 99k LAN Rate Mode is a per-instance configurable feature that enables high-bandwidth LAN server functionality using NAT-based iptables rules. This allows servers to bypass the default 25k rate limit for clients connecting over the internet. +The 99k LAN Rate Mode is a per-instance option that lets QLDS offer LAN-rate +settings to internet clients. On hosts migrated to the hook mechanism +(`Host.lan_rate_uses_hook = true`), QLSM enables this by activating the reserved +system hook `force_rate.so` through the same LD_PRELOAD/system-hook pipeline used +for hook maintenance. On legacy hosts, QLSM falls back to the older NAT-based +iptables path. ### Overview When enabled, LAN rate mode: 1. Configures the QLDS server with LAN-specific settings (`sv_serverType 1`, `sv_lanForceRate 1`) -2. Sets up NAT iptables rules to redirect external traffic through localhost -3. Enables the `route_localnet` kernel parameter to allow routing to 127.0.0.1 +2. On migrated hosts, enables the `force_rate.so` system hook when templating the service unit +3. On legacy hosts, applies NAT/`route_localnet` rules so external clients appear as LAN clients When disabled (default for internet servers): 1. Configures the QLDS server for internet mode (`sv_serverType 2`, `sv_lanForceRate 0`) -2. No NAT rules are applied - traffic goes directly to the server +2. Removes the migrated-host system-hook predicate or legacy NAT behavior from the active service config ### Server Arguments @@ -384,43 +393,13 @@ When disabled (default for internet servers): +set sv_serverType 2 +set sv_lanForceRate 0 ``` -### Network Configuration - -The LAN rate mode uses iptables NAT rules to make external clients appear as localhost connections: - -**NAT Rules (per port with LAN rate enabled):** -```bash -# PREROUTING: Redirect incoming packets to localhost -iptables -t nat -A PREROUTING -p udp --dport -j DNAT --to-destination 127.0.0.1 - -# POSTROUTING: Source NAT for responses -iptables -t nat -A POSTROUTING -p udp -d 127.0.0.1 --dport -j SNAT --to-source 127.0.0.1 - -# INPUT (required for NAT to work properly) -iptables -t nat -A INPUT -d 127.0.0.1 -j SNAT --to-source 127.0.0.1 -``` - -**Kernel Parameter:** -```bash -sysctl -w net.ipv4.conf.all.route_localnet=1 -``` - ### Implementation Details -- **Database:** `lan_rate_enabled` boolean field on `QLInstance` model (default: `false`) -- **API Endpoint:** `PUT /api/instances//lan-rate` to toggle LAN rate on existing instances -- **Ansible Playbooks:** - - `add_qlds_instance.yml`: Conditionally sets up route_localnet when deploying with LAN rate enabled - - `update_instance_lan_rate.yml`: Toggles LAN rate on existing instances (updates systemd service, adds/removes NAT rules, restarts service) -- **Task Logic:** `reconfigure_instance_lan_rate_logic()` in `ui/task_logic/ansible_instance_mgmt.py` - -### Host-Wide Settings - -The `route_localnet` sysctl setting is host-wide. Once enabled for any instance with LAN rate, it remains enabled. This is safe because: -- Enabling it when not needed is harmless -- Disabling it when any instance still needs it would break those instances - -The NAT iptables rules are per-instance (per-port) and are added/removed individually. +- **Database:** `lan_rate_enabled` boolean field on `QLInstance` model (default: `false`); `Host.lan_rate_uses_hook` selects migrated hook-based handling versus the legacy NAT path. +- **API Endpoint:** `PUT /api/instances//lan-rate` toggles LAN rate on existing instances. +- **Migrated host path:** `reconfigure_instance_lan_rate_logic()` delegates to `apply_instance_hooks_logic(..., restart_service=True)`, which re-renders the unit with current LD_PRELOAD paths and restarts the instance. +- **Legacy host path:** `update_instance_lan_rate.yml` updates systemd arguments, reconciles per-instance NAT rules, and restarts the service. +- **Host setup/migration:** setup and migration tasks install/sync `force_rate.so` and mark hosts as hook-capable when the system-hook path is available. ### Frontend diff --git a/docs/user/features/hooks.md b/docs/user/features/hooks.md index 1cd9615..f9b0fd5 100644 --- a/docs/user/features/hooks.md +++ b/docs/user/features/hooks.md @@ -46,7 +46,7 @@ Reordering is also saved through **Save Configuration**. ### Delete A Hook -Click the delete (trash) icon on the hook row and confirm in the modal. Deleting a hook removes it from the host on the next sync. +Click the delete (trash) icon on the hook row and confirm in the modal. Deleting a hook removes it from the game host on the next **Save Configuration** sync. Deletes happen immediately on the QLSM server. The removed binary and any resulting `LD_PRELOAD` changes are reflected on the game host the next time you click **Save Configuration**. diff --git a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx index 26ecccf..309967f 100644 --- a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx +++ b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx @@ -456,11 +456,13 @@ function EditInstanceConfigModal({ [hookEnabledOrder, initialHookEnabledOrder], ); const hooksForceRestart = hooksDirty && instanceStatus === 'running'; - const restartForced = lanRateChanged || hooksForceRestart; + const hooksKeepStopped = hooksDirty && instanceStatus === 'stopped' && !lanRateChanged; + const restartForced = lanRateChanged || hooksForceRestart || hooksKeepStopped; useEffect(() => { if (hooksForceRestart) setRestartAfterSave(true); - }, [hooksForceRestart]); + if (hooksKeepStopped) setRestartAfterSave(false); + }, [hooksForceRestart, hooksKeepStopped]); const handleRestartToggle = () => { if (restartForced) return; @@ -965,7 +967,15 @@ function EditInstanceConfigModal({ Restart after saving {restartForced && ( - + )} diff --git a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx index bd7681b..752b142 100644 --- a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx +++ b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx @@ -452,14 +452,14 @@ describe('EditInstanceConfigModal preset saving', () => { expect(screen.getByText('Changing hooks requires an instance restart')).toBeInTheDocument(); }); - it('does NOT force restart when hooks change on a STOPPED instance', async () => { + it('keeps restart off and disabled when hooks change on a STOPPED instance', async () => { mocks.getInstanceById.mockResolvedValue({ host_name: 'h', lan_rate_enabled: false, status: 'stopped', name: 'i' }); render( ); @@ -467,7 +467,9 @@ describe('EditInstanceConfigModal preset saving', () => { await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); fireEvent.click(screen.getByText('Mock Toggle c.so')); const toggle = screen.getByRole('button', { name: /toggle restart after save/i }); - expect(toggle).not.toBeDisabled(); + await waitFor(() => expect(toggle).toBeDisabled()); + expect(toggle).toHaveAttribute('aria-pressed', 'false'); + expect(screen.getByText('Stopped instances stay stopped when hook changes are saved')).toBeInTheDocument(); }); it('does NOT send enabled_hooks when the hooks fetch failed on open', async () => { diff --git a/tests/test_instance_hooks_routes.py b/tests/test_instance_hooks_routes.py index 1672bbf..aa32410 100644 --- a/tests/test_instance_hooks_routes.py +++ b/tests/test_instance_hooks_routes.py @@ -99,9 +99,8 @@ def test_get_ignores_legacy_scripts_so_files(client, headers, instance_with_scri def test_legacy_hook_in_scripts_still_applies_but_not_listed( app, client, headers, instance_with_scripts, tmp_path, monkeypatch ): - """Pre-existing instances with a .so only in scripts/ must continue to Apply - (via LD_PRELOAD resolver fallback) but must NOT appear in GET /hooks.""" - from unittest.mock import patch, MagicMock + """Pre-existing instances with a .so only in scripts/ must continue to resolve + through the LD_PRELOAD fallback but must NOT appear in GET /hooks.""" from ui.task_logic import ansible_instance_mgmt scripts = tmp_path / "configs" / "h1" / str(instance_with_scripts.id) / "scripts" From 7984a64e5efb4bb674e6c35ee6a84d8c5babb0bc Mon Sep 17 00:00:00 2001 From: rage Date: Sun, 5 Jul 2026 13:47:46 -0700 Subject: [PATCH 07/10] fix(hooks): enforce save semantics across API path --- docs/api_reference.md | 35 ++++------ .../instances/EditInstanceConfigModal.jsx | 12 +++- .../src/components/instances/HooksTab.jsx | 2 +- .../instances/__tests__/HooksTab.test.jsx | 12 +++- tests/test_instance_api_routes.py | 66 +++++++++++++++++++ tests/test_task_apply_config.py | 23 +++++++ ui/routes/instance_routes.py | 19 +++++- ui/task_logic/ansible_instance_mgmt.py | 7 +- ui/tasks.py | 3 +- 9 files changed, 146 insertions(+), 33 deletions(-) diff --git a/docs/api_reference.md b/docs/api_reference.md index fef2ba2..5b8a36a 100644 --- a/docs/api_reference.md +++ b/docs/api_reference.md @@ -278,7 +278,6 @@ Example success response: | `/instances//config` | GET | Get instance config files | | `/instances//config` | PUT | Update config and apply (triggers Ansible sync) | | `/instances//hooks` | GET | List available LD_PRELOAD hook shared objects | -| `/instances//hooks` | PUT | Replace enabled LD_PRELOAD hook list and queue apply | | `/instances//hooks/files` | POST | Upload a new hook `.so` file | | `/instances//hooks/files/` | GET/PUT/PATCH/DELETE | Download, replace, rename, or delete a hook file | | `/instances//hooks/files//description` | PATCH | Set a hook file description | @@ -423,37 +422,25 @@ BinaryMetadata description, and active system hooks. } ``` -``` -PUT /api/instances//hooks -``` - -Replaces the ordered LD_PRELOAD hook list. Files in `scripts/` that are absent -from `enabled` are disabled. +Hook enable/order changes are saved through the shared config endpoint: -```json -{ - "enabled": ["highfps_hook.so", "timer_hook.so"] -} +```http +PUT /api/instances//config ``` -Validation requires each filename to be a unique `.so` basename with no path -separators, no `..`, not reserved for a system hook, present under `user-hooks/` -(or `scripts/` for legacy instances), and an ELF file. Validation failures -return `400`; held instance locks return `409`. - -Success returns `202 Accepted` and queues `apply_instance_hooks`. - ```json { - "data": { - "task_id": "rq-job-id" - } + "configs": { "server.cfg": "...", "mappool.txt": "...", "access.txt": "...", "workshop.txt": "..." }, + "enabled_hooks": ["highfps_hook.so", "timer_hook.so"], + "restart": true } ``` -Running instances restart after the unit is re-rendered with -`Environment=LD_PRELOAD=...`; stopped instances update the unit and remain -stopped. +`enabled_hooks` replaces the ordered LD_PRELOAD hook list after filtering to +`.so` basenames that exist in the instance `user-hooks/` directory. Running +instances with changed hooks are forced to restart even if a client submits +`"restart": false`; stopped instances with hook-only changes are applied with +`restart=false` and remain stopped. ### Hook File CRUD diff --git a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx index 309967f..3c70bd3 100644 --- a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx +++ b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx @@ -141,6 +141,7 @@ function EditInstanceConfigModal({ const [hookSystem, setHookSystem] = useState([]); const [hookEnabledOrder, setHookEnabledOrder] = useState([]); const [initialHookEnabledOrder, setInitialHookEnabledOrder] = useState([]); + const [hookDiskChanged, setHookDiskChanged] = useState(false); const [hooksLoaded, setHooksLoaded] = useState(false); const [instanceStatus, setInstanceStatus] = useState(null); @@ -310,6 +311,7 @@ function EditInstanceConfigModal({ setHookSystem([]); setHookEnabledOrder([]); setInitialHookEnabledOrder([]); + setHookDiskChanged(false); setHooksLoaded(false); setInstanceStatus(null); pluginsSyncedRef.current = false; @@ -451,9 +453,10 @@ function EditInstanceConfigModal({ const checkedPluginsChanged = !setsEqual(checkedPlugins, initialCheckedPlugins); const hooksDirty = useMemo( - () => hookEnabledOrder.length !== initialHookEnabledOrder.length + () => hookDiskChanged + || hookEnabledOrder.length !== initialHookEnabledOrder.length || hookEnabledOrder.some((filename, index) => initialHookEnabledOrder[index] !== filename), - [hookEnabledOrder, initialHookEnabledOrder], + [hookDiskChanged, hookEnabledOrder, initialHookEnabledOrder], ); const hooksForceRestart = hooksDirty && instanceStatus === 'running'; const hooksKeepStopped = hooksDirty && instanceStatus === 'stopped' && !lanRateChanged; @@ -737,7 +740,10 @@ function EditInstanceConfigModal({ setHookEnabledOrder((cur) => cur.filter((name) => name !== filename)); }, []); - const handleRefreshHooks = useCallback(() => loadHooks(), [loadHooks]); + const handleRefreshHooks = useCallback((options = {}) => { + if (options.hooksChanged) setHookDiskChanged(true); + return loadHooks(); + }, [loadHooks]); const handleExpandEditor = (selectedFile, content = '') => { const fileNameToExpand = typeof selectedFile === 'string' diff --git a/frontend-react/src/components/instances/HooksTab.jsx b/frontend-react/src/components/instances/HooksTab.jsx index c8b97fe..e384a54 100644 --- a/frontend-react/src/components/instances/HooksTab.jsx +++ b/frontend-react/src/components/instances/HooksTab.jsx @@ -51,7 +51,7 @@ export default function HooksTab({ setError(null); try { await uploadInstanceHook(instanceId, file); - onRefresh?.(); + onRefresh?.({ hooksChanged: enabledSet.has(file.name) }); } catch (err) { setError(errorMessage(err, 'Upload failed.')); } finally { diff --git a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx index c4b61d1..e1075ef 100644 --- a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx +++ b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx @@ -94,7 +94,17 @@ describe('HooksTab', () => { fireEvent.change(screen.getByTestId('hook-upload-input'), { target: { files: [file] } }); await waitFor(() => expect(api.uploadInstanceHook).toHaveBeenCalledWith(1, file)); - expect(props.onRefresh).toHaveBeenCalledTimes(1); + expect(props.onRefresh).toHaveBeenCalledWith({ hooksChanged: false }); + }); + + it('uploading over an enabled hook marks hook files changed for restart forcing', async () => { + const { props } = renderTab(); + + const file = new File(['ELF content'], 'a.so', { type: 'application/octet-stream' }); + fireEvent.change(screen.getByTestId('hook-upload-input'), { target: { files: [file] } }); + + await waitFor(() => expect(api.uploadInstanceHook).toHaveBeenCalledWith(1, file)); + expect(props.onRefresh).toHaveBeenCalledWith({ hooksChanged: true }); }); it('shows error banner when upload fails', async () => { diff --git a/tests/test_instance_api_routes.py b/tests/test_instance_api_routes.py index c1e2991..5acca6e 100644 --- a/tests/test_instance_api_routes.py +++ b/tests/test_instance_api_routes.py @@ -598,6 +598,72 @@ def test_update_config_enabled_hooks_without_draft_filters_to_existing_files( assert updated.ld_preload_hooks == 'existing_hook.so' +def test_update_config_forces_restart_for_running_hook_changes( + client, app, auth_token, sample_instance, tmp_path, monkeypatch +): + monkeypatch.chdir(tmp_path) + instance, host = sample_instance + hooks_dir = tmp_path / 'configs' / host.name / str(instance.id) / 'user-hooks' + hooks_dir.mkdir(parents=True) + (hooks_dir / 'existing_hook.so').write_bytes(b'\x7fELF' + b'\x00' * 16) + + with app.app_context(): + db_instance = db.session.get(QLInstance, instance.id) + db_instance.status = InstanceStatus.RUNNING + db.session.commit() + + payload = { + 'configs': _full_configs(), + 'enabled_hooks': ['existing_hook.so'], + 'restart': False, + } + + with patch('ui.routes.instance_routes.acquire_lock', return_value=True), \ + patch('ui.routes.instance_routes.enqueue_task', return_value=MagicMock(id='job-1')) as enqueue: + response = client.put( + f'/api/instances/{instance.id}/config', + json=payload, + headers=_auth_header(auth_token), + ) + + assert response.status_code == 202, response.get_json() + assert enqueue.call_args.kwargs['restart'] is True + assert enqueue.call_args.kwargs['previous_status'] == 'running' + + +def test_update_config_keeps_stopped_hook_changes_stopped( + client, app, auth_token, sample_instance, tmp_path, monkeypatch +): + monkeypatch.chdir(tmp_path) + instance, host = sample_instance + hooks_dir = tmp_path / 'configs' / host.name / str(instance.id) / 'user-hooks' + hooks_dir.mkdir(parents=True) + (hooks_dir / 'existing_hook.so').write_bytes(b'\x7fELF' + b'\x00' * 16) + + with app.app_context(): + db_instance = db.session.get(QLInstance, instance.id) + db_instance.status = InstanceStatus.STOPPED + db.session.commit() + + payload = { + 'configs': _full_configs(), + 'enabled_hooks': ['existing_hook.so'], + 'restart': True, + } + + with patch('ui.routes.instance_routes.acquire_lock', return_value=True), \ + patch('ui.routes.instance_routes.enqueue_task', return_value=MagicMock(id='job-1')) as enqueue: + response = client.put( + f'/api/instances/{instance.id}/config', + json=payload, + headers=_auth_header(auth_token), + ) + + assert response.status_code == 202, response.get_json() + assert enqueue.call_args.kwargs['restart'] is False + assert enqueue.call_args.kwargs['previous_status'] == 'stopped' + + def test_update_config_creates_user_hooks_source_dir_when_absent( client, app, auth_token, sample_instance, tmp_path, monkeypatch ): diff --git a/tests/test_task_apply_config.py b/tests/test_task_apply_config.py index 0f7d983..76db28a 100644 --- a/tests/test_task_apply_config.py +++ b/tests/test_task_apply_config.py @@ -273,3 +273,26 @@ def test_apply_instance_config_pip_warning_no_restart( assert mock_instance.status == InstanceStatus.UPDATED logged_messages = [str(call) for call in mock_append_log.call_args_list] assert any('pip install failed' in msg for msg in logged_messages) + + +@patch(f'{TASK_LOGIC_MODULE}._run_ansible_playbook') +@patch(f'{TASK_LOGIC_MODULE}._build_qlds_args_string', return_value='mock_qlds_args') +@patch(f'{TASK_LOGIC_MODULE}._prepare_instance_zmq') +@patch(f'{TASK_LOGIC_MODULE}.append_log') +@patch(f'{TASK_LOGIC_MODULE}.db.session') +@patch(f'{TASK_LOGIC_MODULE}.get_current_job') +def test_apply_instance_config_previous_stopped_status_remains_stopped_without_restart( + mock_get_job, mock_session, mock_append_log, mock_prep_zmq, + mock_build_args, mock_run_playbook, test_app +): + mock_job = MagicMock(); mock_job.id = 'test-job-id' + mock_get_job.return_value = mock_job + + mock_instance = _make_mock_instance(status=InstanceStatus.CONFIGURING) + mock_session.get.return_value = mock_instance + mock_run_playbook.return_value = (SimpleAnsibleResult(0, 'ok', ''), None) + + result = apply_instance_config(12, restart=False, previous_status='stopped') + + assert mock_instance.status == InstanceStatus.STOPPED + assert 'Status: stopped' in result diff --git a/ui/routes/instance_routes.py b/ui/routes/instance_routes.py index 76a5764..541a4c5 100644 --- a/ui/routes/instance_routes.py +++ b/ui/routes/instance_routes.py @@ -965,6 +965,10 @@ def manage_instance_config_api(instance_id): # Renamed and combined GET/POST fro if not _draft_exists(draft_id): return jsonify({"error": {"message": "Draft not found. It may have expired."}}), 400 + original_status = instance.status + original_ld_preload_hooks = instance.ld_preload_hooks or None + original_lan_rate_enabled = instance.lan_rate_enabled + update_kwargs = dict(status=InstanceStatus.CONFIGURING) update_kwargs.update(metadata_update_kwargs) if 'lan_rate_enabled' in data: @@ -1044,9 +1048,12 @@ def manage_instance_config_api(instance_id): # Renamed and combined GET/POST fro f.write(content) current_app.logger.info(f"Saved updated scripts for instance {instance.id}") + hooks_changed = False if enabled_hooks_data is not None: filtered_hooks = _filtered_enabled_hooks(enabled_hooks_data, instance_user_hooks_dir) - update_kwargs['ld_preload_hooks'] = ",".join(filtered_hooks) if filtered_hooks else None + next_ld_preload_hooks = ",".join(filtered_hooks) if filtered_hooks else None + hooks_changed = next_ld_preload_hooks != original_ld_preload_hooks + update_kwargs['ld_preload_hooks'] = next_ld_preload_hooks # Handle factories updates when the client sends the full factories map. if factories_present: @@ -1061,12 +1068,22 @@ def manage_instance_config_api(instance_id): # Renamed and combined GET/POST fro return jsonify({"error": {"message": f"An instance with the name '{new_name}' already exists."}}), 409 restart = data.get('restart', True) + lan_rate_changed = ( + 'lan_rate_enabled' in data + and bool(data.get('lan_rate_enabled')) != original_lan_rate_enabled + ) + if hooks_changed: + if original_status == InstanceStatus.RUNNING: + restart = True + elif original_status == InstanceStatus.STOPPED and not lan_rate_changed: + restart = False reconcile_lan_rate_network = 'lan_rate_enabled' in data job = enqueue_task( apply_instance_config, instance.id, restart=restart, reconcile_lan_rate_network=reconcile_lan_rate_network, + previous_status=original_status.value, lock_token=lock_token, on_failure=instance_job_failure_handler, ) diff --git a/ui/task_logic/ansible_instance_mgmt.py b/ui/task_logic/ansible_instance_mgmt.py index 0ed5130..1f2e29b 100644 --- a/ui/task_logic/ansible_instance_mgmt.py +++ b/ui/task_logic/ansible_instance_mgmt.py @@ -535,7 +535,7 @@ def start_instance_logic(instance_id): return f"Error during instance {instance_id} start: {e}" -def apply_instance_config_logic(instance_id, restart=True, reconcile_lan_rate_network=False): +def apply_instance_config_logic(instance_id, restart=True, reconcile_lan_rate_network=False, previous_status=None): """ Logic for applying configuration to a QL instance via Ansible. This involves syncing config files and optionally restarting the service. @@ -558,7 +558,10 @@ def apply_instance_config_logic(instance_id, restart=True, reconcile_lan_rate_ne db.session.commit() return f"Error during instance {instance.id} config apply: Host not found" - original_status = instance.status # Store original status + if previous_status is not None: + original_status = InstanceStatus(previous_status) + else: + original_status = instance.status append_log(instance, f"Task started: apply_instance_config (Job ID: {job_id}, Restart: {restart})") instance.status = InstanceStatus.CONFIGURING db.session.commit() diff --git a/ui/tasks.py b/ui/tasks.py index abca62d..0245b81 100644 --- a/ui/tasks.py +++ b/ui/tasks.py @@ -180,13 +180,14 @@ def start_instance(instance_id, lock_token=None): @rq.job(timeout=300) @with_app_context -def apply_instance_config(instance_id, restart=True, reconcile_lan_rate_network=False, lock_token=None): +def apply_instance_config(instance_id, restart=True, reconcile_lan_rate_network=False, previous_status=None, lock_token=None): """RQ task entry point for applying configuration to a QL instance.""" try: return apply_instance_config_logic( instance_id, restart=restart, reconcile_lan_rate_network=reconcile_lan_rate_network, + previous_status=previous_status, ) finally: if lock_token: From 97ca9b9d008c3e6c417a715c1e40b60e976ad00c Mon Sep 17 00:00:00 2001 From: rage Date: Mon, 6 Jul 2026 11:51:39 -0700 Subject: [PATCH 08/10] fix(hooks): force restart when replacing an enabled hook binary The per-row Replace flow called onChanged?.() with no args, so replacing an enabled hook's binary on a running instance left hooksDirty false and did not force the restart toggle. The new .so would not load until a later manual restart, violating the running-hook-change restart invariant. Propagate { hooksChanged: hook.enabled } from both replace handlers, mirroring the upload-over-enabled path in HooksTab.handleUpload. --- frontend-react/src/components/instances/HookRow.jsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend-react/src/components/instances/HookRow.jsx b/frontend-react/src/components/instances/HookRow.jsx index 8629b1c..cbadbc0 100644 --- a/frontend-react/src/components/instances/HookRow.jsx +++ b/frontend-react/src/components/instances/HookRow.jsx @@ -134,7 +134,9 @@ function HookActionsMenu({ hook, instanceId, onChanged, onDelete, onRename }) { setActionError(null); try { await replaceInstanceHook(instanceId, hook.filename, file); - onChanged?.(); + // Replacing an enabled hook's binary is a hook change: flag it so a running + // instance is forced to restart on the next Save Configuration. + onChanged?.({ hooksChanged: hook.enabled }); } catch (err) { setActionError(err?.error?.message || 'Replace failed'); } @@ -235,7 +237,9 @@ function HookRowContent({ hook, onToggle, dragHandleProps = null, style = undefi setActionError(null); try { await replaceInstanceHook(instanceId, hook.filename, file); - onChanged?.(); + // Replacing an enabled hook's binary is a hook change: flag it so a running + // instance is forced to restart on the next Save Configuration. + onChanged?.({ hooksChanged: hook.enabled }); } catch (err) { setActionError(err?.error?.message || 'Replace failed'); } From f13bac9ae7a993608b7804da4cf3bc2ac0996f8b Mon Sep 17 00:00:00 2001 From: rage Date: Mon, 6 Jul 2026 11:57:05 -0700 Subject: [PATCH 09/10] test(hooks): cover replace-hook restart-forcing signal Assert the per-row Replace flow propagates { hooksChanged } based on the hook's enabled state: true for an enabled hook (forces restart), false for a disabled one. Verified both fail without the HookRow.jsx fix. --- .../instances/__tests__/HooksTab.test.jsx | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx index e1075ef..f4fb896 100644 --- a/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx +++ b/frontend-react/src/components/instances/__tests__/HooksTab.test.jsx @@ -33,6 +33,7 @@ describe('HooksTab', () => { beforeEach(() => { vi.clearAllMocks(); api.uploadInstanceHook.mockResolvedValue({}); + api.replaceInstanceHook.mockResolvedValue({}); api.deleteInstanceHook.mockResolvedValue({}); api.renameInstanceHook.mockResolvedValue({}); api.setInstanceHookDescription.mockResolvedValue({}); @@ -107,6 +108,30 @@ describe('HooksTab', () => { expect(props.onRefresh).toHaveBeenCalledWith({ hooksChanged: true }); }); + it('replacing an ENABLED hook binary marks hook files changed for restart forcing', async () => { + const { props } = renderTab(); + + const row = screen.getByTestId('hook-row-a.so'); + const input = row.querySelector('input[type="file"]'); + const file = new File(['ELF content'], 'a.so', { type: 'application/octet-stream' }); + fireEvent.change(input, { target: { files: [file] } }); + + await waitFor(() => expect(api.replaceInstanceHook).toHaveBeenCalledWith(1, 'a.so', file)); + expect(props.onRefresh).toHaveBeenCalledWith({ hooksChanged: true }); + }); + + it('replacing a DISABLED hook binary does not force a restart', async () => { + const { props } = renderTab(); + + const row = screen.getByTestId('hook-row-c.so'); + const input = row.querySelector('input[type="file"]'); + const file = new File(['ELF content'], 'c.so', { type: 'application/octet-stream' }); + fireEvent.change(input, { target: { files: [file] } }); + + await waitFor(() => expect(api.replaceInstanceHook).toHaveBeenCalledWith(1, 'c.so', file)); + expect(props.onRefresh).toHaveBeenCalledWith({ hooksChanged: false }); + }); + it('shows error banner when upload fails', async () => { api.uploadInstanceHook.mockRejectedValueOnce({ error: { message: 'Not an ELF file' } }); renderTab(); From a8922043c2ad259dcacf78761eba82c0e8d1ed28 Mon Sep 17 00:00:00 2001 From: rage Date: Tue, 7 Jul 2026 08:26:29 -0700 Subject: [PATCH 10/10] feat(hooks): add preset Load/Save buttons to Hooks tab Show the Load Preset and Save Preset buttons on the Hooks tab like every other tab in the instance config editor. Save/Overwrite Preset now capture the live hook draft (enablement + order) instead of re-fetching persisted instance hooks, so a preset saved from the Hooks tab reflects the current selection. Bump v1.13.8. --- VERSION | 2 +- docs/user/features/hooks.md | 4 +- docs/user/releases.md | 1 + docs/user/version.json | 2 +- .../instances/EditInstanceConfigModal.jsx | 60 ++++++++----------- .../EditInstanceConfigModal.test.jsx | 26 ++++++++ 6 files changed, 57 insertions(+), 38 deletions(-) diff --git a/VERSION b/VERSION index b0f139e..237a256 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.13.7 +1.13.8 diff --git a/docs/user/features/hooks.md b/docs/user/features/hooks.md index f9b0fd5..4687640 100644 --- a/docs/user/features/hooks.md +++ b/docs/user/features/hooks.md @@ -73,7 +73,9 @@ When an instance starts, QLSM builds the `LD_PRELOAD` value in this order: ## Hooks In Presets -Saving a preset from an instance also records which user hooks were enabled (and their load order). Loading that preset onto another instance and saving replaces the target instance's enabled hooks to match — see [Presets And Default Config](../presets/overview.md). +The **Load Preset** and **Save Preset** buttons are available on the Hooks tab, the same as every other tab in the editor. + +Saving a preset from an instance also records which user hooks were enabled (and their load order) — capturing the current selection shown in the Hooks tab, including changes you haven't saved yet. Loading that preset onto another instance and saving replaces the target instance's enabled hooks to match — see [Presets And Default Config](../presets/overview.md). ## Related Pages diff --git a/docs/user/releases.md b/docs/user/releases.md index 883461c..1e98b3a 100644 --- a/docs/user/releases.md +++ b/docs/user/releases.md @@ -4,6 +4,7 @@ QLSM uses `v..` tags. Every merged pull request is listed a | Version | Date | PR | Changes | | --- | --- | --- | --- | +| `v1.13.8` | 2026-07-07 | — | Add Load Preset and Save Preset buttons to the Hooks tab; Save Preset captures the current hook selection. | | `v1.13.7` | 2026-07-05 | — | Hooks now save through Save Configuration; removed the separate Apply & Restart button. | | `v1.13.6` | 2026-07-03 | [#136](https://github.com/dngrtech/qlsm/pull/136) | Fix presets losing their LD_PRELOAD hook files when loaded onto an instance, and make presets remember which hooks were enabled. | | `v1.13.5` | 2026-07-03 | [#135](https://github.com/dngrtech/qlsm/pull/135) | Fix preset ZIP import rejecting `.so` plugin scripts, and surface `.so` scripts correctly in the preset API instead of silently dropping them. | diff --git a/docs/user/version.json b/docs/user/version.json index 3f5f6f4..2ad6a24 100644 --- a/docs/user/version.json +++ b/docs/user/version.json @@ -1,4 +1,4 @@ { - "latest": "1.13.7", + "latest": "1.13.8", "releaseNotesUrl": "https://dngrtech.github.io/qlsm/releases/" } diff --git a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx index 3c70bd3..9f43f7d 100644 --- a/frontend-react/src/components/instances/EditInstanceConfigModal.jsx +++ b/frontend-react/src/components/instances/EditInstanceConfigModal.jsx @@ -48,10 +48,6 @@ const getFactoryLinterSource = (fileName) => ( fileName?.toLowerCase().endsWith('.factories') ? FACTORY_LINTER_SOURCE : null ); const getServerHostname = (serverCfg = '') => serverCfg.match(/set sv_hostname "([^"]*)"/)?.[1] || ''; -const enabledHookFilenames = (hooksData) => (hooksData.available || []) - .filter((hook) => hook.enabled) - .sort((a, b) => a.order - b.order) - .map((hook) => hook.filename); const setsEqual = (a, b) => a.size === b.size && [...a].every(value => b.has(value)); // Mapping between frontend config keys and backend preset API keys @@ -572,10 +568,10 @@ function EditInstanceConfigModal({ presetData.checked_plugins = Array.from(checkedPlugins); } - try { - presetData.enabled_hooks = enabledHookFilenames(await fetchInstanceHooks(instanceId)); - } catch { - // Best-effort: skip enabled_hooks if the fetch fails, don't block preset save + // Capture the live hook draft (enablement + order) so a preset saved from + // the Hooks tab reflects the current selection, matching Save Configuration. + if (hooksLoaded) { + presetData.enabled_hooks = hookEnabledOrder; } presetData.binary_meta_source = { @@ -601,7 +597,7 @@ function EditInstanceConfigModal({ } finally { setIsSavingPreset(false); } - }, [checkedPlugins, instanceId, pluginDraftId, serializeConfigs, serializeFactories, showSuccess, showError]); + }, [checkedPlugins, hookEnabledOrder, hooksLoaded, instanceId, pluginDraftId, serializeConfigs, serializeFactories, showSuccess, showError]); const handleOverwritePreset = useCallback(async (presetId, { description }) => { setIsSavingPreset(true); @@ -623,10 +619,8 @@ function EditInstanceConfigModal({ presetData.draft_id = pluginDraftId; presetData.checked_plugins = Array.from(checkedPlugins); } - try { - presetData.enabled_hooks = enabledHookFilenames(await fetchInstanceHooks(instanceId)); - } catch { - // Best-effort: skip enabled_hooks if the fetch fails, don't block preset save + if (hooksLoaded) { + presetData.enabled_hooks = hookEnabledOrder; } presetData.binary_meta_source = { context_type: 'instance', context_key: String(instanceId) }; const response = await updatePreset(presetId, presetData); @@ -641,7 +635,7 @@ function EditInstanceConfigModal({ } finally { setIsSavingPreset(false); } - }, [checkedPlugins, instanceId, pluginDraftId, serializeConfigs, serializeFactories, showSuccess, showError]); + }, [checkedPlugins, hookEnabledOrder, hooksLoaded, instanceId, pluginDraftId, serializeConfigs, serializeFactories, showSuccess, showError]); const handlePresetDeleted = useCallback((deletedPresetId) => { setPresets(prevPresets => prevPresets.filter(p => p.id !== deletedPresetId)); @@ -1086,28 +1080,24 @@ function EditInstanceConfigModal({
- {/* Left side - Preset management buttons (non-hooks tabs only) */} + {/* Left side - Preset management buttons */}
- {activeMainTab !== 'hooks' && ( - <> - - - - )} + +
{/* Right side - Esc hint + Cancel + Save */} diff --git a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx index 752b142..0f0d257 100644 --- a/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx +++ b/frontend-react/src/components/instances/__tests__/EditInstanceConfigModal.test.jsx @@ -432,6 +432,32 @@ describe('EditInstanceConfigModal preset saving', () => { expect(mocks.updateInstanceConfig.mock.calls[0][1].enabled_hooks).toEqual(['a.so', 'c.so']); }); + it('shows the preset buttons on the hooks tab and Save Preset captures the live hook draft', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByText('hooks-tab')).toBeInTheDocument()); + // Preset buttons must be present on the hooks tab, same as every other tab. + expect(screen.getByRole('button', { name: /load preset/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /save preset/i })).toBeInTheDocument(); + + // A hook toggle made on this tab must be reflected in the saved preset. + fireEvent.click(screen.getByRole('button', { name: /mock toggle c.so/i })); + fireEvent.click(screen.getByRole('button', { name: /save preset/i })); + fireEvent.click(screen.getByRole('button', { name: /confirm save preset/i })); + + await waitFor(() => expect(mocks.createPreset).toHaveBeenCalledTimes(1)); + expect(mocks.createPreset.mock.calls[0][0].enabled_hooks).toEqual(['a.so', 'c.so']); + }); + it('forces + disables restart toggle when hooks change on a RUNNING instance', async () => { mocks.getInstanceById.mockResolvedValue({ host_name: 'h', lan_rate_enabled: false, status: 'running', name: 'i' });