From e2e2e66f995545a7efb8aeef98ac9b51f70de5d4 Mon Sep 17 00:00:00 2001
From: dtzp555 <dtzp555@gmail.com>
Date: Mon, 11 May 2026 03:37:08 +1000
Subject: [PATCH] fix(setup): plist-merge code-quality follow-up

Three issues raised by the code-quality reviewer on 0fd1838:

1. mergeSystemdEnv now early-returns when the template has no
   Environment= anchor, instead of silently dropping preserved lines
   (defensive guard for a future template change).
2. Both parsers (parsePlistEnv, parseSystemdEnv) now Buffer-normalise
   their input, removing the TypeError-vs-coerce asymmetry.
3. Two idempotency tests added (calling merge twice on the same
   input/output pair must be stable).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 scripts/lib/plist-merge.mjs |  6 ++++++
 test-features.mjs           | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/scripts/lib/plist-merge.mjs b/scripts/lib/plist-merge.mjs
index 0cd55f4..52683f5 100644
--- a/scripts/lib/plist-merge.mjs
+++ b/scripts/lib/plist-merge.mjs
@@ -13,6 +13,7 @@ const PLIST_KV_RE = /<key>([^<]+)<\/key>\s*<string>([^<]*)<\/string>/g;
 
 export function parsePlistEnv(plistContent) {
   if (!plistContent) return {};
+  if (Buffer.isBuffer(plistContent)) plistContent = plistContent.toString("utf8");
   // Restrict to the EnvironmentVariables dict to avoid catching Label, etc.
   const envBlock = plistContent.match(/<key>EnvironmentVariables<\/key>\s*<dict>([\s\S]*?)<\/dict>/);
   if (!envBlock) return {};
@@ -52,6 +53,7 @@ const SYSTEMD_KV_RE = /^Environment=([^=]+)=(.*)$/gm;
 
 export function parseSystemdEnv(serviceContent) {
   if (!serviceContent) return {};
+  if (Buffer.isBuffer(serviceContent)) serviceContent = serviceContent.toString("utf8");
   const out = {};
   let m;
   SYSTEMD_KV_RE.lastIndex = 0;
@@ -72,6 +74,10 @@ export function mergeSystemdEnv(existing, template) {
     .map(([k, v]) => `Environment=${k}=${v}`);
   if (preservedLines.length === 0) return template;
 
+  // Guard: if template has no Environment= anchor, cannot inject — return template as-is.
+  // (In practice the OCP systemd template always has Environment= lines.)
+  if (!/^Environment=/m.test(template)) return template;
+
   // Inject after the last existing Environment= line in the template
   return template.replace(
     /(^Environment=[^\n]+\n)((?!Environment=).*$)/ms,
diff --git a/test-features.mjs b/test-features.mjs
index 5f31573..5d9b2b6 100644
--- a/test-features.mjs
+++ b/test-features.mjs
@@ -564,6 +564,16 @@ test("mergeSystemdEnv first-install returns template unchanged", () => {
   assert.equal(mergeSystemdEnv("", SAMPLE_TEMPLATE_SYSTEMD), SAMPLE_TEMPLATE_SYSTEMD);
 });
 
+test("mergePlistEnv is idempotent", () => {
+  const r1 = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
+  assert.equal(mergePlistEnv(r1, SAMPLE_TEMPLATE_PLIST), r1);
+});
+
+test("mergeSystemdEnv is idempotent", () => {
+  const r1 = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
+  assert.equal(mergeSystemdEnv(r1, SAMPLE_TEMPLATE_SYSTEMD), r1);
+});
+
 // ── Cleanup ──
 closeDb();