mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
fix(setup): merge plist/systemd env vars instead of overwriting
setup.mjs previously wrote the launchd plist and the Linux systemd unit with writeFileSync(path, NEW_TEMPLATE_STRING), which silently dropped any user-customised env vars (CLAUDE_HEARTBEAT_INTERVAL, CLAUDE_CACHE_TTL, etc.) on every re-setup or upgrade. This change introduces scripts/lib/plist-merge.mjs which preserves keys present only in the existing file and lets the template's known keys win. Linux variant uses the same logic against Environment=KEY=VALUE lines. Tests added in test-features.mjs cover preserve / override / first-install for both formats. No cli.js citation needed: this is OCP-internal installer behaviour with no corresponding cli.js operation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,80 @@
|
|||||||
|
// scripts/lib/plist-merge.mjs
|
||||||
|
//
|
||||||
|
// Preserves user-customised env vars when setup.mjs rewrites the unit file.
|
||||||
|
//
|
||||||
|
// Rule:
|
||||||
|
// - keys present in NEW template → template value wins (template is source of truth)
|
||||||
|
// - keys ONLY in EXISTING (not in template) → preserved verbatim
|
||||||
|
//
|
||||||
|
// No new dependencies — regex-based, plist <key>X</key><string>Y</string> shape
|
||||||
|
// is stable enough for our hand-written templates in setup.mjs.
|
||||||
|
|
||||||
|
const PLIST_KV_RE = /<key>([^<]+)<\/key>\s*<string>([^<]*)<\/string>/g;
|
||||||
|
|
||||||
|
export function parsePlistEnv(plistContent) {
|
||||||
|
if (!plistContent) return {};
|
||||||
|
// 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 {};
|
||||||
|
const out = {};
|
||||||
|
let m;
|
||||||
|
PLIST_KV_RE.lastIndex = 0;
|
||||||
|
while ((m = PLIST_KV_RE.exec(envBlock[1])) !== null) {
|
||||||
|
out[m[1]] = m[2];
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function mergePlistEnv(existing, template) {
|
||||||
|
if (!existing) return template;
|
||||||
|
const existingEnv = parsePlistEnv(existing);
|
||||||
|
const templateEnv = parsePlistEnv(template);
|
||||||
|
const KNOWN = new Set(Object.keys(templateEnv));
|
||||||
|
|
||||||
|
const preserved = {};
|
||||||
|
for (const [k, v] of Object.entries(existingEnv)) {
|
||||||
|
if (!KNOWN.has(k)) preserved[k] = v;
|
||||||
|
}
|
||||||
|
if (Object.keys(preserved).length === 0) return template;
|
||||||
|
|
||||||
|
const lines = Object.entries(preserved)
|
||||||
|
.map(([k, v]) => ` <key>${k}</key>\n <string>${v}</string>`)
|
||||||
|
.join("\n");
|
||||||
|
|
||||||
|
// Inject before the closing </dict> of EnvironmentVariables
|
||||||
|
return template.replace(
|
||||||
|
/(<key>EnvironmentVariables<\/key>\s*<dict>[\s\S]*?)(\n\s*<\/dict>)/,
|
||||||
|
`$1\n${lines}$2`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const SYSTEMD_KV_RE = /^Environment=([^=]+)=(.*)$/gm;
|
||||||
|
|
||||||
|
export function parseSystemdEnv(serviceContent) {
|
||||||
|
if (!serviceContent) return {};
|
||||||
|
const out = {};
|
||||||
|
let m;
|
||||||
|
SYSTEMD_KV_RE.lastIndex = 0;
|
||||||
|
while ((m = SYSTEMD_KV_RE.exec(serviceContent)) !== null) {
|
||||||
|
out[m[1]] = m[2];
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function mergeSystemdEnv(existing, template) {
|
||||||
|
if (!existing) return template;
|
||||||
|
const existingEnv = parseSystemdEnv(existing);
|
||||||
|
const templateEnv = parseSystemdEnv(template);
|
||||||
|
const KNOWN = new Set(Object.keys(templateEnv));
|
||||||
|
|
||||||
|
const preservedLines = Object.entries(existingEnv)
|
||||||
|
.filter(([k]) => !KNOWN.has(k))
|
||||||
|
.map(([k, v]) => `Environment=${k}=${v}`);
|
||||||
|
if (preservedLines.length === 0) return template;
|
||||||
|
|
||||||
|
// Inject after the last existing Environment= line in the template
|
||||||
|
return template.replace(
|
||||||
|
/(^Environment=[^\n]+\n)((?!Environment=).*$)/ms,
|
||||||
|
`$1${preservedLines.join("\n")}\n$2`
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -13,6 +13,7 @@
|
|||||||
* 5. Optionally starts the proxy
|
* 5. Optionally starts the proxy
|
||||||
*/
|
*/
|
||||||
import { readFileSync, writeFileSync, existsSync, mkdirSync, unlinkSync, readdirSync, chmodSync } from "node:fs";
|
import { readFileSync, writeFileSync, existsSync, mkdirSync, unlinkSync, readdirSync, chmodSync } from "node:fs";
|
||||||
|
import { mergePlistEnv, mergeSystemdEnv } from "./scripts/lib/plist-merge.mjs";
|
||||||
import { execSync } from "node:child_process";
|
import { execSync } from "node:child_process";
|
||||||
import { join, dirname } from "node:path";
|
import { join, dirname } from "node:path";
|
||||||
import { homedir } from "node:os";
|
import { homedir } from "node:os";
|
||||||
@@ -424,9 +425,15 @@ if (!DRY_RUN) {
|
|||||||
</plist>
|
</plist>
|
||||||
`;
|
`;
|
||||||
|
|
||||||
writeFileSync(plistPath, plistXml);
|
const existingPlist = existsSync(plistPath) ? readFileSync(plistPath, "utf8") : null;
|
||||||
|
const finalPlistXml = mergePlistEnv(existingPlist, plistXml);
|
||||||
|
writeFileSync(plistPath, finalPlistXml);
|
||||||
chmodSync(plistPath, 0o600);
|
chmodSync(plistPath, 0o600);
|
||||||
log(`Plist written: ${plistPath} (mode 600)`);
|
if (existingPlist && finalPlistXml !== plistXml) {
|
||||||
|
log(`Plist written: ${plistPath} (mode 600, preserved user env vars)`);
|
||||||
|
} else {
|
||||||
|
log(`Plist written: ${plistPath} (mode 600)`);
|
||||||
|
}
|
||||||
|
|
||||||
// Bootout first (in case it was already loaded) then bootstrap
|
// Bootout first (in case it was already loaded) then bootstrap
|
||||||
try { execSync(`launchctl bootout gui/$(id -u) "${plistPath}" 2>/dev/null`); } catch { /* ignore */ }
|
try { execSync(`launchctl bootout gui/$(id -u) "${plistPath}" 2>/dev/null`); } catch { /* ignore */ }
|
||||||
@@ -459,9 +466,15 @@ StandardError=append:${logPath}
|
|||||||
WantedBy=default.target
|
WantedBy=default.target
|
||||||
`;
|
`;
|
||||||
|
|
||||||
writeFileSync(servicePath, serviceUnit);
|
const existingService = existsSync(servicePath) ? readFileSync(servicePath, "utf8") : null;
|
||||||
|
const finalServiceUnit = mergeSystemdEnv(existingService, serviceUnit);
|
||||||
|
writeFileSync(servicePath, finalServiceUnit);
|
||||||
chmodSync(servicePath, 0o600);
|
chmodSync(servicePath, 0o600);
|
||||||
log(`Service file written: ${servicePath} (mode 600)`);
|
if (existingService && finalServiceUnit !== serviceUnit) {
|
||||||
|
log(`Service file written: ${servicePath} (mode 600, preserved user env vars)`);
|
||||||
|
} else {
|
||||||
|
log(`Service file written: ${servicePath} (mode 600)`);
|
||||||
|
}
|
||||||
|
|
||||||
execSync(`systemctl --user daemon-reload`);
|
execSync(`systemctl --user daemon-reload`);
|
||||||
execSync(`systemctl --user enable ocp-proxy`);
|
execSync(`systemctl --user enable ocp-proxy`);
|
||||||
|
|||||||
@@ -454,6 +454,116 @@ async function runSingleflightTests() {
|
|||||||
|
|
||||||
await runSingleflightTests();
|
await runSingleflightTests();
|
||||||
|
|
||||||
|
// ── Plist Env Merge Tests ──
|
||||||
|
import { mergePlistEnv, mergeSystemdEnv } from "./scripts/lib/plist-merge.mjs";
|
||||||
|
|
||||||
|
console.log("\nPlist env merge:");
|
||||||
|
|
||||||
|
const SAMPLE_TEMPLATE_PLIST = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
|
||||||
|
<plist version="1.0">
|
||||||
|
<dict>
|
||||||
|
<key>Label</key>
|
||||||
|
<string>dev.ocp.proxy</string>
|
||||||
|
<key>EnvironmentVariables</key>
|
||||||
|
<dict>
|
||||||
|
<key>CLAUDE_PROXY_PORT</key>
|
||||||
|
<string>3478</string>
|
||||||
|
<key>CLAUDE_BIND</key>
|
||||||
|
<string>127.0.0.1</string>
|
||||||
|
<key>CLAUDE_AUTH_MODE</key>
|
||||||
|
<string>multi</string>
|
||||||
|
</dict>
|
||||||
|
</dict>
|
||||||
|
</plist>`;
|
||||||
|
|
||||||
|
const SAMPLE_EXISTING_PLIST = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
|
||||||
|
<plist version="1.0">
|
||||||
|
<dict>
|
||||||
|
<key>Label</key>
|
||||||
|
<string>dev.ocp.proxy</string>
|
||||||
|
<key>EnvironmentVariables</key>
|
||||||
|
<dict>
|
||||||
|
<key>CLAUDE_PROXY_PORT</key>
|
||||||
|
<string>3456</string>
|
||||||
|
<key>CLAUDE_BIND</key>
|
||||||
|
<string>127.0.0.1</string>
|
||||||
|
<key>CLAUDE_AUTH_MODE</key>
|
||||||
|
<string>none</string>
|
||||||
|
<key>CLAUDE_HEARTBEAT_INTERVAL</key>
|
||||||
|
<string>2000</string>
|
||||||
|
<key>CLAUDE_CACHE_TTL</key>
|
||||||
|
<string>600</string>
|
||||||
|
</dict>
|
||||||
|
</dict>
|
||||||
|
</plist>`;
|
||||||
|
|
||||||
|
test("mergePlistEnv preserves unknown user keys", () => {
|
||||||
|
const merged = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
|
||||||
|
assert.match(merged, /<key>CLAUDE_HEARTBEAT_INTERVAL<\/key>\s*<string>2000<\/string>/);
|
||||||
|
assert.match(merged, /<key>CLAUDE_CACHE_TTL<\/key>\s*<string>600<\/string>/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("mergePlistEnv overrides known template keys", () => {
|
||||||
|
const merged = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
|
||||||
|
assert.match(merged, /<key>CLAUDE_PROXY_PORT<\/key>\s*<string>3478<\/string>/);
|
||||||
|
assert.match(merged, /<key>CLAUDE_AUTH_MODE<\/key>\s*<string>multi<\/string>/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("mergePlistEnv first-install returns template unchanged when existing is null", () => {
|
||||||
|
const merged = mergePlistEnv(null, SAMPLE_TEMPLATE_PLIST);
|
||||||
|
assert.equal(merged, SAMPLE_TEMPLATE_PLIST);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("mergePlistEnv first-install returns template unchanged when existing is empty", () => {
|
||||||
|
const merged = mergePlistEnv("", SAMPLE_TEMPLATE_PLIST);
|
||||||
|
assert.equal(merged, SAMPLE_TEMPLATE_PLIST);
|
||||||
|
});
|
||||||
|
|
||||||
|
const SAMPLE_TEMPLATE_SYSTEMD = `[Unit]
|
||||||
|
Description=OCP — Open Claude Proxy
|
||||||
|
After=network.target
|
||||||
|
|
||||||
|
[Service]
|
||||||
|
ExecStart=/usr/bin/node /home/u/ocp/server.mjs
|
||||||
|
Environment=CLAUDE_PROXY_PORT=3478
|
||||||
|
Environment=CLAUDE_BIND=127.0.0.1
|
||||||
|
Environment=CLAUDE_AUTH_MODE=multi
|
||||||
|
Restart=always
|
||||||
|
`;
|
||||||
|
|
||||||
|
const SAMPLE_EXISTING_SYSTEMD = `[Unit]
|
||||||
|
Description=OCP — Open Claude Proxy
|
||||||
|
After=network.target
|
||||||
|
|
||||||
|
[Service]
|
||||||
|
ExecStart=/usr/bin/node /home/u/ocp/server.mjs
|
||||||
|
Environment=CLAUDE_PROXY_PORT=3456
|
||||||
|
Environment=CLAUDE_BIND=127.0.0.1
|
||||||
|
Environment=CLAUDE_AUTH_MODE=none
|
||||||
|
Environment=CLAUDE_HEARTBEAT_INTERVAL=2000
|
||||||
|
Environment=CLAUDE_CACHE_TTL=600
|
||||||
|
Restart=always
|
||||||
|
`;
|
||||||
|
|
||||||
|
test("mergeSystemdEnv preserves unknown user Environment lines", () => {
|
||||||
|
const merged = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
|
||||||
|
assert.match(merged, /Environment=CLAUDE_HEARTBEAT_INTERVAL=2000/);
|
||||||
|
assert.match(merged, /Environment=CLAUDE_CACHE_TTL=600/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("mergeSystemdEnv overrides known template keys", () => {
|
||||||
|
const merged = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
|
||||||
|
assert.match(merged, /Environment=CLAUDE_PROXY_PORT=3478/);
|
||||||
|
assert.match(merged, /Environment=CLAUDE_AUTH_MODE=multi/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("mergeSystemdEnv first-install returns template unchanged", () => {
|
||||||
|
assert.equal(mergeSystemdEnv(null, SAMPLE_TEMPLATE_SYSTEMD), SAMPLE_TEMPLATE_SYSTEMD);
|
||||||
|
assert.equal(mergeSystemdEnv("", SAMPLE_TEMPLATE_SYSTEMD), SAMPLE_TEMPLATE_SYSTEMD);
|
||||||
|
});
|
||||||
|
|
||||||
// ── Cleanup ──
|
// ── Cleanup ──
|
||||||
closeDb();
|
closeDb();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user