From 08441639665fd2dbb70db8a89fb981f8ab9ffe26 Mon Sep 17 00:00:00 2001 From: NSK Date: Wed, 22 Jul 2026 04:30:31 +0900 Subject: [PATCH] [defer] fix embedded CLIProxyAPI config handling (#6877) * fix embedded CLIProxyAPI config flag * preserve embedded CLIProxyAPI config * test(services): add fs-backed regression test for cliproxy resolveSpawnArgs (#6877) The existing cliproxy.test.ts only re-asserted string literals and never called the real resolveSpawnArgs() against a filesystem, so it could not have caught the -c/--config flag mismatch or the config.yaml clobbering bug this PR fixes. Add a test that imports the real function against a temp DATA_DIR and asserts: the spawn args always use --config (never -c), a missing config.yaml gets the default template, and an existing operator-customized config.yaml is left byte-identical. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- src/lib/services/installers/cliproxy.ts | 6 +- .../cliproxy-resolve-spawn-args-6877.test.ts | 104 ++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts diff --git a/src/lib/services/installers/cliproxy.ts b/src/lib/services/installers/cliproxy.ts index 61c3b6699b..64c3c01294 100644 --- a/src/lib/services/installers/cliproxy.ts +++ b/src/lib/services/installers/cliproxy.ts @@ -105,11 +105,13 @@ export function resolveSpawnArgs(port: number): SpawnArgs { fs.mkdirSync(CONFIG_DIR, { recursive: true }); const configPath = path.join(CONFIG_DIR, "config.yaml"); - fs.writeFileSync(configPath, `port: ${port}\nhost: 127.0.0.1\nlog_level: warn\n`, "utf8"); + if (!fs.existsSync(configPath)) { + fs.writeFileSync(configPath, `port: ${port}\nhost: 127.0.0.1\nlog_level: warn\n`, "utf8"); + } return { command: symlinkPath, - args: ["-c", configPath], + args: ["--config", configPath], env: { ...process.env }, cwd: CONFIG_DIR, }; diff --git a/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts b/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts new file mode 100644 index 0000000000..db14e2d854 --- /dev/null +++ b/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts @@ -0,0 +1,104 @@ +/** + * Regression test for #6877 — cliproxy resolveSpawnArgs() must: + * (a) invoke the real CLIProxyAPI binary with the long-form `--config` flag + * (the short `-c` flag is not recognized by the upstream binary and + * silently falls back to its own default config, ignoring ours), and + * (b) never clobber an existing config.yaml — only write the default + * template the first time the file does not exist yet. + * + * Unlike the pre-existing tests/unit/services/installers/cliproxy.test.ts + * (which only re-asserts string literals and never calls the real function), + * this test imports and calls the real resolveSpawnArgs() against a real + * temp-directory filesystem. + */ + +import { describe, it, beforeEach, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; + +function createTempDataDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cliproxy-6877-")); +} + +// DATA_DIR is captured into a module-level const the *first* time +// src/lib/db/core.ts (transitively imported by cliproxy.ts) is loaded, and +// dynamic import() results are cached per resolved URL for the lifetime of +// the process — so the env var must be pinned to one fixed temp dir BEFORE +// the module is imported for the first time. Each test then resets the +// *contents* of that same directory rather than re-pointing DATA_DIR. +const FIXED_DATA_DIR = createTempDataDir(); +process.env.DATA_DIR = FIXED_DATA_DIR; + +after(() => { + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } + fs.rmSync(FIXED_DATA_DIR, { recursive: true, force: true }); +}); + +describe("resolveSpawnArgs (#6877 — real filesystem)", () => { + const dataDir = FIXED_DATA_DIR; + + beforeEach(() => { + fs.rmSync(dataDir, { recursive: true, force: true }); + fs.mkdirSync(dataDir, { recursive: true }); + }); + + it("uses the --config long flag and never the -c short flag", async () => { + const { resolveSpawnArgs } = await import( + "../../../../src/lib/services/installers/cliproxy.ts" + ); + + const port = 8317; + const result = resolveSpawnArgs(port); + + const configPath = path.join(dataDir, "services", "cliproxy", "config.yaml"); + + assert.deepEqual(result.args, ["--config", configPath]); + assert.ok(!result.args.includes("-c"), "args must never contain the short -c flag"); + }); + + it("writes the default config.yaml template when none exists yet", async () => { + const { resolveSpawnArgs } = await import( + "../../../../src/lib/services/installers/cliproxy.ts" + ); + + const port = 9123; + const result = resolveSpawnArgs(port); + + const configPath = path.join(dataDir, "services", "cliproxy", "config.yaml"); + assert.equal(result.args[1], configPath); + assert.ok(fs.existsSync(configPath), "config.yaml must be created on first run"); + + const content = fs.readFileSync(configPath, "utf8"); + assert.equal(content, `port: ${port}\nhost: 127.0.0.1\nlog_level: warn\n`); + }); + + it("preserves a pre-existing config.yaml byte-for-byte instead of overwriting it", async () => { + const { resolveSpawnArgs } = await import( + "../../../../src/lib/services/installers/cliproxy.ts" + ); + + const configDir = path.join(dataDir, "services", "cliproxy"); + fs.mkdirSync(configDir, { recursive: true }); + const configPath = path.join(configDir, "config.yaml"); + const customContent = + "port: 8317\nhost: 0.0.0.0\nlog_level: debug\n# hand-edited by the operator\napi-keys:\n - custom-key\n"; + fs.writeFileSync(configPath, customContent, "utf8"); + + resolveSpawnArgs(8317); + + const contentAfter = fs.readFileSync(configPath, "utf8"); + assert.equal( + contentAfter, + customContent, + "resolveSpawnArgs must not clobber an existing, operator-customized config.yaml" + ); + }); +});