Improve config file change handling

src/__tests__/server.test.ts

@@ -1013,6 +1013,34 @@ describe('server', async () => {
 				// and the updated value
 				expect(out).toContain('val:2');
 			});
+			test('preserves blank line between adjacent replaced sections', async () => {
+				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');
+				const content = '[a]\nval: 1\n\n[other]\nfoo: bar\n';
+				const out = replaceOrAddIniSections(content, [
+					{
+						section: 'a',
+						body: 'val: replaced\n',
+					},
+					{
+						section: 'other',
+						body: 'foo: replaced\nbar: baz\n',
+					},
+				]);
+
+				expect(out).toContain('val: replaced\n\n[other]');
+			});
+			test('preserves trailing comments after retained values (including surrounding whitespace) ', async () => {
+				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');
+				const content = '[a]\nx: 1    # comment  \ny: 2   # blah\n\n[other]\nq: 123\n';
+				const out = replaceOrAddIniSections(content, [
+					{
+						section: 'a',
+						body: 'x: 42\n',
+					},
+				]);
+
+				expect(out).toContain('x: 42    # comment  \n\n[other]');
+			});
 			test('multiple updates for same name: last update wins', async () => {
 				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');
 				const content = '';
@@ -1030,8 +1058,26 @@ describe('server', async () => {
 				const out = replaceOrAddIniSections(content, [{ section: 'section1', body: 'v: new\n' }]);
 				// new body should be present
 				expect(out).toContain('v: new');
-				// section2 should still be present
-				expect(out).toContain('[section2]');
+				// comment before section2 should be preserved
+				expect(out).toContain('# Another comment\n[section2]');
+			});
+			test('retains trailing comments after replaced sections at end of content', async () => {
+				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');
+				const content = '# Start comment\n[section1]\nval: 1\n\n# Another comment\n[section2]\nval: 2\n\n# End comment';
+				const out = replaceOrAddIniSections(content, [{ section: 'section2', body: 'v: new\n' }]);
+				// new body should be present
+				expect(out).toContain('v: new');
+				// comment after section2 should be preserved
+				expect(out).toContain('v: new\n\n# End comment');
+			});
+			test('retains trailing comments after unreplaced sections at end of content', async () => {
+				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');
+				const content = '# Start comment\n[section1]\nval: 1\n\n# Another comment\n[section2]\nval: 2\n\n# End comment';
+				const out = replaceOrAddIniSections(content, [{ section: 'section1', body: 'v: new\n' }]);
+				// new body should be present
+				expect(out).toContain('v: new');
+				// comment after section 2 should be preserved
+				expect(out).toContain('val: 2\n\n# End comment');
 			});
 			test('idempotent: replacing section with identical content returns identical output', async () => {
 				const { replaceOrAddIniSections } = await import('@/server/helpers/file-operations');

src/instrumentation.ts

@@ -1,3 +1,5 @@
+import type { PermittedServices } from '@/server/helpers/klipper';
+
 export const register = async () => {
 	if (process.env.NEXT_RUNTIME === 'nodejs') {
 		const { getLogger } = await import('./server/helpers/logger');
@@ -44,7 +46,12 @@ export const register = async () => {
 				});
 				logger.info('Configuration changed, restarting klipper..');
 				try {
-					const restarted = await klipperRestart();
+					const servicesToRestart: PermittedServices[] | undefined = res.some(
+						(r) => r.fileName === 'crowsnest.conf' && (r.action === 'created' || r.action === 'overwritten'),
+					)
+						? ['crowsnest']
+						: undefined;
+					const restarted = await klipperRestart({ servicesToRestart });
 					if (restarted) {
 						logger.info('Klipper restarted!');
 					} else {

src/server/helpers/file-operations.ts

@@ -330,9 +330,42 @@ export function replaceOrAddIniSections(content: string, updates: IniUpdate[]):
 			if (newBodyTrimmed === originalBodyTrimmed) {
 				out += originalBodyWithTrailing;
 			} else {
-				// Body changed: use new body and ensure it ends with newline
-				out += newBody;
-				if (!out.endsWith('\n')) out += '\n';
+				const preserveTrailingInlineComments = (originalBody: string, updatedBody: string) => {
+					const commentByKey = new Map<string, string>();
+					for (const line of originalBody.split('\n')) {
+						const match = line.match(/^\s*([^#;\s][^:]*?)\s*:\s*[^\n]*?(\s+[;#].*)$/);
+						if (!match) continue;
+						const key = match[1].trim();
+						const commentSuffix = match[2];
+						if (!commentByKey.has(key)) {
+							commentByKey.set(key, commentSuffix);
+						}
+					}
+
+					return updatedBody
+						.split('\n')
+						.map((line) => {
+							const keyMatch = line.match(/^\s*([^#;\s][^:]*?)\s*:\s*[^\n]*?$/);
+							if (!keyMatch) return line;
+							if (line.match(/\s+[;#].*$/)) return line;
+							const key = keyMatch[1].trim();
+							const commentSuffix = commentByKey.get(key);
+							if (!commentSuffix) return line;
+							return line + commentSuffix;
+						})
+						.join('\n');
+				};
+
+				const newBodyWithPreservedComments = preserveTrailingInlineComments(originalBodyWithTrailing, newBody);
+				const trailingDecoration =
+					originalBodyWithTrailing.match(/(\n(?:(?:[ \t]*(?:[#;].*)?)\n)*(?:[ \t]*(?:[#;].*)?)?)$/)?.[1] ?? '';
+
+				out += newBodyWithPreservedComments.replace(/\n+$/, '');
+				if (trailingDecoration.length > 0) {
+					out += trailingDecoration;
+				} else if (!out.endsWith('\n')) {
+					out += '\n';
+				}
 			}
 		} else {
 			// copy original section slice exactly as-is

src/server/helpers/klipper.ts

@@ -1,9 +1,17 @@
-import { getLogger } from '@/app/_helpers/logger';
+import { getLogger } from '@/server/helpers/logger';
 import { getErrorMessage } from '@/utils/exception-handling';
 import { MoonrakerPrinterState, MoonrakerPrinterStateErrorEnum, parseMoonrakerHTTPResponse } from '@/zods/moonraker';
-import { get } from 'http';
 import { ZodError } from 'zod';
 
+/**
+ * Query Moonraker for the printer's current print state.
+ *
+ * This function hits Moonraker's `printer/objects/query?print_stats` endpoint and
+ * returns the `state` field from `print_stats`.
+ *
+ * If Moonraker is offline, failing, or returns unexpected data, this function returns
+ * `'error'`.
+ */
 export const queryPrinterState = async (): Promise<
 	Zod.output<typeof MoonrakerPrinterState>['status']['print_stats']['state']
 > => {
@@ -27,13 +35,34 @@ export const queryPrinterState = async (): Promise<
 	return 'error';
 };
 
-export const klipperRestart = async (force = false) => {
-	if (force) {
+/**
+ * Restart Klipper (and optionally other services) via Moonraker.
+ *
+ * This helper will first check the printer state (unless `force` is true) and
+ * will only send a restart request if the printer is in an idle/finished/error
+ * state. When the printer is actively printing, restart will be skipped.
+ *
+ * @param opts.force - If true, skip querying printer state and always attempt restarts.
+ * @param opts.servicesToRestart - Optional list of permitted services to restart before
+ *   triggering the Klipper restart. Service restarts are only attempted if
+ *   the printer state allows for a restart or if `force` is true.
+ * @param opts.abortOnServiceRestartFailure - If true, abort the whole restart process
+ *   when any service restart request fails.
+ * @returns `true` when the klipper restart command was successfully sent, `false` otherwise.
+ *   Note that service restart failures do not affect the return value unless
+ *   `abortOnServiceRestartFailure` is set to true.
+ */
+export const klipperRestart = async (opts?: {
+	force?: boolean;
+	servicesToRestart?: PermittedServices[];
+	abortOnServiceRestartFailure?: boolean;
+}) => {
+	if (opts?.force === true) {
 		getLogger().info('Restarting Klipper without checking printer state...');
 	} else {
 		let state: string | undefined;
 		try {
-			const state = await queryPrinterState();
+			state = await queryPrinterState();
 		} catch (e) {
 			getLogger().error(`Failed to query printer state before Klipper restart: ${getErrorMessage(e)}`);
 			return false;
@@ -45,6 +74,16 @@ export const klipperRestart = async (force = false) => {
 		getLogger().info(`Restarting Klipper, printer is currently in '${state}' state...`);
 	}
 
+	if (opts?.servicesToRestart && opts.servicesToRestart.length > 0) {
+		for (const service of opts.servicesToRestart) {
+			const restarted = await serviceRestart(service);
+			if (!restarted && opts?.abortOnServiceRestartFailure === true) {
+				getLogger().error(`Failed to restart service ${service}, aborting Klipper restart.`);
+				return false;
+			}
+		}
+	}
+
 	try {
 		await fetch('http://localhost:7125/printer/restart', { method: 'POST' });
 		getLogger().info('Klipper restart command sent successfully.');
@@ -55,3 +94,41 @@ export const klipperRestart = async (force = false) => {
 
 	return false;
 };
+
+// Keep in sync with the expected content of moonraker.asvc (see ratos-common.sh)
+export type PermittedServices =
+	| 'klipper_mcu'
+	| 'webcamd'
+	| 'MoonCord'
+	| 'KlipperScreen'
+	| 'moonraker-telegram-bot'
+	| 'moonraker-obico'
+	| 'sonar'
+	| 'crowsnest'
+	| 'octoeverywhere'
+	| 'ratos-configurator';
+
+/**
+ * Restart a permitted service through Moonraker.
+ *
+ * This simply sends a restart request to Moonraker's service API.
+ *
+ * @param service - One of the allowed services defined by `PermittedServices`.
+ * @returns `true` when the restart request was sent successfully, `false` on error.
+ */
+export const serviceRestart = async (service: PermittedServices) => {
+	getLogger().info(`Attempting to restart service ${service} via Moonraker...`);
+	try {
+		await fetch('http://localhost:7125/machine/services/restart', {
+			method: 'POST',
+			headers: { 'Content-Type': 'application/json' },
+			body: JSON.stringify({ service }),
+		});
+		getLogger().info(`${service} restart command sent successfully.`);
+		return true;
+	} catch (e) {
+		getLogger().error(`Failed to send ${service} restart command: ${getErrorMessage(e)}`);
+	}
+
+	return false;
+};

src/server/routers/printer.ts

@@ -76,7 +76,7 @@ import { ToolheadHelper, ToolheadSuffix } from '@/helpers/toolhead';
 import { getLastPrinterSettings, hasLastPrinterSettings, savePrinterSettings } from '@/server/helpers/printer-settings';
 import { PrinterAxis } from '@/zods/motion';
 import { ServerCache, cacheAsyncDirectoryFn } from '@/server/helpers/cache';
-import { klipperRestart } from '@/server/helpers/klipper';
+import { klipperRestart, PermittedServices, serviceRestart } from '@/server/helpers/klipper';
 import { access, copyFile, readFile, unlink, writeFile } from 'fs/promises';
 import { exec } from 'child_process';
 import objectHash from 'object-hash';
@@ -567,6 +567,19 @@ const generateKlipperConfiguration = async <T extends boolean>(
 				// At this point we know the file exists.
 				if (file.overwrite) {
 					if (file.exists && file.diskContent === file.content) {
+						// Ensure that last saved file is up to date. This can happen if the desired content has
+						// intentionally been read from the current disk content (eg. some of the
+						// VAOC files do this to achieve "create if it doesn't exist, otherwise keep unchanged" behaviour ).
+						// If we don't do this, the last saved content can get out of sync with the actual content on disk,
+						// which can cause confusion later when we compare desired content with last saved content to
+						// determine if a file has been changed on disk.
+						if (file.lastSavedContent !== file.content) {
+							const lastSavedDir = path.dirname(lastSavedPath);
+							if (!existsSync(lastSavedDir)) {
+								mkdirSync(lastSavedDir, { recursive: true });
+							}
+							await writeFile(lastSavedPath, file.content);
+						}
 						return { fileName: file.fileName, action: 'unchanged' };
 					}
 					// Make a back up.
@@ -582,8 +595,8 @@ const generateKlipperConfiguration = async <T extends boolean>(
 						);
 						if (backups.length > 0) {
 							const sortedBackups = backups.sort((a, b) => {
-								const aDate = new Date(a.split('-').slice(-1)[0].split('.cfg')[0]);
-								const bDate = new Date(b.split('-').slice(-1)[0].split('.cfg')[0]);
+								const aDate = new Date(a.split('-').slice(-1)[0].split(fileExt)[0]);
+								const bDate = new Date(b.split('-').slice(-1)[0].split(fileExt)[0]);
 								return aDate.getTime() - bDate.getTime();
 							});
 							if (sortedBackups.length > BACKUPS_TO_KEEP) {
@@ -1158,7 +1171,12 @@ export const printerRouter = router({
 		.mutation(async ({ input }) => {
 			const res = await regenerateKlipperConfiguration(undefined, input.overwriteFiles, input.skipFiles);
 			if (res.some((r) => r.action === 'created' || r.action === 'overwritten')) {
-				klipperRestart();
+				const servicesToRestart: PermittedServices[] | undefined = res.some(
+					(r) => r.fileName === 'crowsnest.conf' && (r.action === 'created' || r.action === 'overwritten'),
+				)
+					? ['crowsnest']
+					: undefined;
+				klipperRestart({ servicesToRestart });
 			}
 			return res;
 		}),
@@ -1184,8 +1202,13 @@ export const printerRouter = router({
 		.mutation(async (ctx) => {
 			const { config: serializedConfig, overwriteFiles, skipFiles } = ctx.input;
 			const config = await deserializePrinterConfiguration(serializedConfig);
-			const configResult = await generateKlipperConfiguration(config, overwriteFiles, skipFiles);
-			klipperRestart();
+			const configResult = await generateKlipperConfiguration<false>(config, overwriteFiles, skipFiles);
+			const servicesToRestart: PermittedServices[] | undefined = configResult.some(
+				(r) => r.fileName === 'crowsnest.conf' && (r.action === 'created' || r.action === 'overwritten'),
+			)
+				? ['crowsnest']
+				: undefined;
+			klipperRestart({ servicesToRestart });
 			return configResult;
 		}),
 	flashBeacon: publicProcedure.mutation(async () => {