Skip to content

Commit d23ad17

Browse files
fallintoplacefallintoplace
committed
fix: make proxy config writing idempotent
1 parent e0fdf01 commit d23ad17

3 files changed

Lines changed: 430 additions & 40 deletions

File tree

‎dist/main.js‎

Lines changed: 84 additions & 17 deletions
Large diffs are not rendered by default.

‎src/writeProxyConfig.ts‎

Lines changed: 100 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import { SafetyStrategy } from "./runCodexExec";
55
import { checkOutput } from "./checkOutput";
66

77
const MODEL_PROVIDER = "codex-action-responses-proxy";
8+
const MANAGED_MODEL_PROVIDER_START =
9+
"# BEGIN action-managed model provider config";
10+
const MANAGED_MODEL_PROVIDER_END =
11+
"# END action-managed model provider config";
12+
const MANAGED_PROXY_PROVIDER_START =
13+
"# BEGIN action-managed proxy provider config";
14+
const MANAGED_PROXY_PROVIDER_END =
15+
"# END action-managed proxy provider config";
16+
const LINE_BREAK = "\\r?\\n";
817

918
export async function writeProxyConfig(
1019
codexHome: string,
@@ -13,29 +22,14 @@ export async function writeProxyConfig(
1322
): Promise<void> {
1423
const configPath = path.join(codexHome, "config.toml");
1524

16-
let existing = "";
17-
try {
18-
existing = await fs.readFile(configPath, "utf8");
19-
} catch {
20-
existing = "";
21-
}
22-
23-
const header = `# Added by codex-action.
24-
model_provider = "${MODEL_PROVIDER}"
25-
26-
27-
`;
28-
const table = `
29-
30-
# Added by codex-action.
31-
[model_providers.${MODEL_PROVIDER}]
32-
name = "Codex Action Responses Proxy"
33-
base_url = "http://127.0.0.1:${port}/v1"
34-
wire_api = "responses"
35-
`;
36-
37-
// Prepend model_provider at the very top.
38-
let output = `${header}${existing}${table}`;
25+
const existing = await readExistingConfig(configPath);
26+
const unmanagedConfig = stripManagedProxyConfig(existing);
27+
const managedModelProvider = renderManagedModelProviderConfig();
28+
const managedProxyProvider = renderManagedProxyProviderConfig(port);
29+
const output =
30+
unmanagedConfig.length > 0
31+
? `${managedModelProvider}\n\n${unmanagedConfig}\n\n${managedProxyProvider}\n`
32+
: `${managedModelProvider}\n\n${managedProxyProvider}\n`;
3933

4034
if (safetyStrategy === "unprivileged-user") {
4135
// We know we have already created the CODEX_HOME directory, but it is owned
@@ -53,3 +47,86 @@ wire_api = "responses"
5347
await fs.writeFile(configPath, output, "utf8");
5448
}
5549
}
50+
51+
async function readExistingConfig(configPath: string): Promise<string> {
52+
try {
53+
return await fs.readFile(configPath, "utf8");
54+
} catch (error) {
55+
if (
56+
error instanceof Error &&
57+
"code" in error &&
58+
(error as NodeJS.ErrnoException).code === "ENOENT"
59+
) {
60+
return "";
61+
}
62+
throw error;
63+
}
64+
}
65+
66+
function renderManagedModelProviderConfig(): string {
67+
return `${MANAGED_MODEL_PROVIDER_START}
68+
model_provider = "${MODEL_PROVIDER}"
69+
${MANAGED_MODEL_PROVIDER_END}`;
70+
}
71+
72+
function renderManagedProxyProviderConfig(port: number): string {
73+
return `${MANAGED_PROXY_PROVIDER_START}
74+
[model_providers.${MODEL_PROVIDER}]
75+
name = "Codex Action Responses Proxy"
76+
base_url = "http://127.0.0.1:${port}/v1"
77+
wire_api = "responses"
78+
${MANAGED_PROXY_PROVIDER_END}`;
79+
}
80+
81+
function stripManagedProxyConfig(existing: string): string {
82+
let output = existing;
83+
84+
output = stripManagedSection(
85+
output,
86+
MANAGED_MODEL_PROVIDER_START,
87+
MANAGED_MODEL_PROVIDER_END
88+
);
89+
output = stripManagedSection(
90+
output,
91+
MANAGED_PROXY_PROVIDER_START,
92+
MANAGED_PROXY_PROVIDER_END
93+
);
94+
95+
output = output.replace(
96+
new RegExp(
97+
`^# Added by codex-action\\.${LINE_BREAK}model_provider = "${escapeRegExp(
98+
MODEL_PROVIDER
99+
)}"(?:${LINE_BREAK}|$)(?:${LINE_BREAK})*`,
100+
"gm"
101+
),
102+
""
103+
);
104+
105+
output = output.replace(
106+
new RegExp(
107+
`(?:${LINE_BREAK})*# Added by codex-action\\.${LINE_BREAK}\\[model_providers\\.${escapeRegExp(
108+
MODEL_PROVIDER
109+
)}\\]${LINE_BREAK}name = "Codex Action Responses Proxy"${LINE_BREAK}base_url = "http:\\/\\/127\\.0\\.0\\.1:\\d+\\/v1"${LINE_BREAK}wire_api = "responses"(?:${LINE_BREAK}|$)(?:${LINE_BREAK})*`,
110+
"g"
111+
),
112+
"\n"
113+
);
114+
115+
return output.trim();
116+
}
117+
118+
function stripManagedSection(existing: string, start: string, end: string): string {
119+
return existing.replace(
120+
new RegExp(
121+
`^${escapeRegExp(start)}${LINE_BREAK}[\\s\\S]*?^${escapeRegExp(
122+
end
123+
)}(?:${LINE_BREAK}|$)(?:${LINE_BREAK})*`,
124+
"gm"
125+
),
126+
""
127+
);
128+
}
129+
130+
function escapeRegExp(value: string): string {
131+
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
132+
}

‎test/writeProxyConfig.test.mjs‎

Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
import assert from "node:assert/strict";
2+
import { spawnSync } from "node:child_process";
3+
import * as fs from "node:fs/promises";
4+
import * as os from "node:os";
5+
import * as path from "node:path";
6+
import { test } from "node:test";
7+
import { fileURLToPath } from "node:url";
8+
9+
const mainPath = fileURLToPath(new URL("../dist/main.js", import.meta.url));
10+
const MODEL_PROVIDER = "codex-action-responses-proxy";
11+
const MANAGED_MODEL_PROVIDER_START =
12+
"# BEGIN action-managed model provider config";
13+
const MANAGED_MODEL_PROVIDER_END =
14+
"# END action-managed model provider config";
15+
const MANAGED_PROXY_PROVIDER_START =
16+
"# BEGIN action-managed proxy provider config";
17+
const MANAGED_PROXY_PROVIDER_END =
18+
"# END action-managed proxy provider config";
19+
20+
function runWriteProxyConfig(home, port) {
21+
const result = spawnSync(
22+
process.execPath,
23+
[
24+
mainPath,
25+
"write-proxy-config",
26+
"--codex-home",
27+
home,
28+
"--port",
29+
String(port),
30+
"--safety-strategy",
31+
"unsafe",
32+
],
33+
{ encoding: "utf8" }
34+
);
35+
36+
assert.equal(result.status, 0, result.stderr || result.stdout);
37+
}
38+
39+
async function withTempHome(fn) {
40+
const home = await fs.mkdtemp(path.join(os.tmpdir(), "write-proxy-config-"));
41+
try {
42+
await fn(home);
43+
} finally {
44+
await fs.rm(home, { recursive: true, force: true });
45+
}
46+
}
47+
48+
async function readConfig(home) {
49+
return await fs.readFile(path.join(home, "config.toml"), "utf8");
50+
}
51+
52+
function countOccurrences(value, search) {
53+
return value.split(search).length - 1;
54+
}
55+
56+
test("creates config.toml when no config exists", async () => {
57+
await withTempHome(async (home) => {
58+
runWriteProxyConfig(home, 1234);
59+
60+
const config = await readConfig(home);
61+
62+
assert.match(
63+
config,
64+
new RegExp(`^${escapeRegExp(MANAGED_MODEL_PROVIDER_START)}\\n`)
65+
);
66+
assert.match(
67+
config,
68+
new RegExp(`\\n${escapeRegExp(MANAGED_PROXY_PROVIDER_END)}\\n$`)
69+
);
70+
assert.match(config, new RegExp(`model_provider = "${MODEL_PROVIDER}"`));
71+
assert.match(config, /base_url = "http:\/\/127\.0\.0\.1:1234\/v1"/);
72+
});
73+
});
74+
75+
test("keeps one provider block when run twice", async () => {
76+
await withTempHome(async (home) => {
77+
runWriteProxyConfig(home, 1234);
78+
runWriteProxyConfig(home, 5678);
79+
80+
const config = await readConfig(home);
81+
82+
assert.equal(countOccurrences(config, MANAGED_MODEL_PROVIDER_START), 1);
83+
assert.equal(countOccurrences(config, MANAGED_MODEL_PROVIDER_END), 1);
84+
assert.equal(countOccurrences(config, MANAGED_PROXY_PROVIDER_START), 1);
85+
assert.equal(countOccurrences(config, MANAGED_PROXY_PROVIDER_END), 1);
86+
assert.equal(
87+
countOccurrences(config, `model_provider = "${MODEL_PROVIDER}"`),
88+
1
89+
);
90+
assert.equal(
91+
countOccurrences(config, `[model_providers.${MODEL_PROVIDER}]`),
92+
1
93+
);
94+
});
95+
});
96+
97+
test("updates the proxy port when run twice with a new port", async () => {
98+
await withTempHome(async (home) => {
99+
runWriteProxyConfig(home, 1234);
100+
runWriteProxyConfig(home, 5678);
101+
102+
const config = await readConfig(home);
103+
104+
assert.doesNotMatch(config, /127\.0\.0\.1:1234/);
105+
assert.match(config, /base_url = "http:\/\/127\.0\.0\.1:5678\/v1"/);
106+
});
107+
});
108+
109+
test("preserves unrelated config when writing proxy config", async () => {
110+
await withTempHome(async (home) => {
111+
await fs.mkdir(home, { recursive: true });
112+
await fs.writeFile(
113+
path.join(home, "config.toml"),
114+
`model = "gpt-5"
115+
approval_policy = "never"
116+
117+
[profiles.ci]
118+
model = "gpt-5"
119+
`,
120+
"utf8"
121+
);
122+
123+
runWriteProxyConfig(home, 1234);
124+
125+
const config = await readConfig(home);
126+
const rootConfigIndex = config.indexOf('model = "gpt-5"');
127+
const providerTableIndex = config.indexOf(
128+
`[model_providers.${MODEL_PROVIDER}]`
129+
);
130+
131+
assert.match(config, /^# BEGIN action-managed model provider config/);
132+
assert.match(
133+
config,
134+
/model = "gpt-5"\napproval_policy = "never"\n\n\[profiles\.ci\]\nmodel = "gpt-5"/
135+
);
136+
assert.ok(rootConfigIndex > -1);
137+
assert.ok(providerTableIndex > rootConfigIndex);
138+
assert.equal(countOccurrences(config, `[profiles.ci]`), 1);
139+
assert.equal(
140+
countOccurrences(config, `[model_providers.${MODEL_PROVIDER}]`),
141+
1
142+
);
143+
});
144+
});
145+
146+
test("migrates old managed format when existing config used old comments", async () => {
147+
await withTempHome(async (home) => {
148+
await fs.mkdir(home, { recursive: true });
149+
await fs.writeFile(
150+
path.join(home, "config.toml"),
151+
`# Added by codex-action.
152+
model_provider = "${MODEL_PROVIDER}"
153+
154+
155+
# Added by codex-action.
156+
model_provider = "${MODEL_PROVIDER}"
157+
158+
159+
[profiles.ci]
160+
model = "gpt-5"
161+
approval_policy = "never"
162+
163+
164+
# Added by codex-action.
165+
[model_providers.${MODEL_PROVIDER}]
166+
name = "Codex Action Responses Proxy"
167+
base_url = "http://127.0.0.1:1111/v1"
168+
wire_api = "responses"
169+
170+
171+
# Added by codex-action.
172+
[model_providers.${MODEL_PROVIDER}]
173+
name = "Codex Action Responses Proxy"
174+
base_url = "http://127.0.0.1:2222/v1"
175+
wire_api = "responses"
176+
`,
177+
"utf8"
178+
);
179+
180+
runWriteProxyConfig(home, 3333);
181+
182+
const config = await readConfig(home);
183+
184+
assert.doesNotMatch(config, /# Added by codex-action\./);
185+
assert.equal(countOccurrences(config, MANAGED_MODEL_PROVIDER_START), 1);
186+
assert.equal(countOccurrences(config, MANAGED_PROXY_PROVIDER_START), 1);
187+
assert.equal(
188+
countOccurrences(config, `model_provider = "${MODEL_PROVIDER}"`),
189+
1
190+
);
191+
assert.equal(
192+
countOccurrences(config, `[model_providers.${MODEL_PROVIDER}]`),
193+
1
194+
);
195+
assert.match(
196+
config,
197+
/\[profiles\.ci\]\nmodel = "gpt-5"\napproval_policy = "never"/
198+
);
199+
assert.ok(
200+
config.indexOf("[profiles.ci]") <
201+
config.indexOf(`[model_providers.${MODEL_PROVIDER}]`)
202+
);
203+
assert.match(config, /base_url = "http:\/\/127\.0\.0\.1:3333\/v1"/);
204+
});
205+
});
206+
207+
test("migrates compact old managed format with CRLF line endings", async () => {
208+
await withTempHome(async (home) => {
209+
await fs.mkdir(home, { recursive: true });
210+
const config = [
211+
"# Added by codex-action.",
212+
`model_provider = "${MODEL_PROVIDER}"`,
213+
"# Added by codex-action.",
214+
`model_provider = "${MODEL_PROVIDER}"`,
215+
"[profiles.ci]",
216+
'model = "gpt-5"',
217+
"# Added by codex-action.",
218+
`[model_providers.${MODEL_PROVIDER}]`,
219+
'name = "Codex Action Responses Proxy"',
220+
'base_url = "http://127.0.0.1:1111/v1"',
221+
'wire_api = "responses"',
222+
].join("\r\n");
223+
224+
await fs.writeFile(path.join(home, "config.toml"), config, "utf8");
225+
226+
runWriteProxyConfig(home, 3333);
227+
228+
const updatedConfig = await readConfig(home);
229+
230+
assert.doesNotMatch(updatedConfig, /# Added by codex-action\./);
231+
assert.equal(
232+
countOccurrences(updatedConfig, `model_provider = "${MODEL_PROVIDER}"`),
233+
1
234+
);
235+
assert.equal(
236+
countOccurrences(updatedConfig, `[model_providers.${MODEL_PROVIDER}]`),
237+
1
238+
);
239+
assert.match(updatedConfig, /\[profiles\.ci\]\r?\nmodel = "gpt-5"/);
240+
assert.match(updatedConfig, /base_url = "http:\/\/127\.0\.0\.1:3333\/v1"/);
241+
});
242+
});
243+
244+
function escapeRegExp(value) {
245+
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
246+
}

0 commit comments

Comments
 (0)