mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
fix(setup): preserve user-customised plist/systemd env vars on re-setup (#90)
* 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>
* 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>
---------
Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -454,6 +454,126 @@ async function 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);
|
||||
});
|
||||
|
||||
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();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user