Skip to content

Commit b99e012

Browse files
SurviveMSurviveM
committed
fix: verify run port cleanup targets
1 parent 04af762 commit b99e012

4 files changed

Lines changed: 178 additions & 39 deletions

File tree

‎cli.js‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -237,28 +237,36 @@ function releaseRunPortIfNeeded(port, deps = {}) {
237237
? deps.kill
238238
: (typeof processRef.kill === 'function' ? processRef.kill.bind(processRef) : null);
239239
const seenPids = new Set();
240+
const candidatePids = new Set();
240241
const currentPid = Number(processRef.pid);
241242
let released = false;
242243

243-
const addPidsFromText = (text) => {
244+
const addPidsFromText = (text, targetSet = seenPids) => {
245+
if (!targetSet) {
246+
return;
247+
}
244248
const lines = String(text || '').split(/\r?\n/);
245249
for (const line of lines) {
246250
const trimmed = line.trim();
247251
if (!/^\d+$/.test(trimmed)) {
248252
continue;
249253
}
250-
seenPids.add(Number(trimmed));
254+
targetSet.add(Number(trimmed));
251255
}
252256
};
253257

254-
const runCommand = (command, args) => {
258+
const runCommand = (command, args, options = {}) => {
259+
const {
260+
stdoutPidSet = seenPids,
261+
stderrPidSet = seenPids
262+
} = options;
255263
const result = runSpawnSync(command, args, { encoding: 'utf8' });
256-
if (result && result.stdout) addPidsFromText(result.stdout);
257-
if (result && result.stderr) addPidsFromText(result.stderr);
264+
if (result && result.stdout) addPidsFromText(result.stdout, stdoutPidSet);
265+
if (result && result.stderr) addPidsFromText(result.stderr, stderrPidSet);
258266
return result || {};
259267
};
260268

261-
const addManagedRunPidsFromPs = (text) => {
269+
const addManagedRunPidsFromPs = (text, allowedPids = null) => {
262270
const lines = String(text || '').split(/\r?\n/);
263271
for (const line of lines) {
264272
const normalizedLine = ` ${line.replace(/\s+/g, ' ').trim()} `;
@@ -273,6 +281,9 @@ function releaseRunPortIfNeeded(port, deps = {}) {
273281
if (!Number.isFinite(pid) || pid <= 0 || pid === currentPid) {
274282
continue;
275283
}
284+
if (allowedPids && !allowedPids.has(pid)) {
285+
continue;
286+
}
276287
seenPids.add(pid);
277288
}
278289
};
@@ -302,23 +313,31 @@ function releaseRunPortIfNeeded(port, deps = {}) {
302313
}
303314
}
304315
} else {
305-
const fuserResult = runCommand('fuser', ['-k', `${numericPort}/tcp`]);
306-
if (!fuserResult.error && fuserResult.status === 0) {
307-
released = true;
308-
}
309-
const fuserMissing = !!(fuserResult && fuserResult.error && fuserResult.error.code === 'ENOENT');
310-
if ((fuserMissing || !released || seenPids.size === 0) && killProcess) {
311-
const lsofResult = runCommand('lsof', ['-ti', `tcp:${numericPort}`]);
312-
if (!(lsofResult && lsofResult.error)) {
313-
// noop: lsof pid collection is handled through stdout parsing above.
316+
let psResult = null;
317+
const readPsResult = () => {
318+
if (psResult) {
319+
return psResult;
314320
}
315-
}
316-
}
321+
psResult = runCommand('ps', ['-ef'], { stdoutPidSet: null, stderrPidSet: null });
322+
return psResult;
323+
};
317324

318-
if (processRef.platform !== 'win32' && killProcess && !released && seenPids.size === 0) {
319-
const psResult = runCommand('ps', ['-ef']);
320-
if (!(psResult && psResult.error)) {
321-
addManagedRunPidsFromPs(psResult.stdout);
325+
const lsofResult = runCommand(
326+
'lsof',
327+
['-ti', `tcp:${numericPort}`],
328+
{ stdoutPidSet: candidatePids, stderrPidSet: null }
329+
);
330+
if (!(lsofResult && lsofResult.error) && candidatePids.size > 0) {
331+
const managedPsResult = readPsResult();
332+
if (!(managedPsResult && managedPsResult.error)) {
333+
addManagedRunPidsFromPs(managedPsResult.stdout, candidatePids);
334+
}
335+
}
336+
if (killProcess && seenPids.size === 0) {
337+
const managedPsResult = readPsResult();
338+
if (!(managedPsResult && managedPsResult.error)) {
339+
addManagedRunPidsFromPs(managedPsResult.stdout);
340+
}
322341
}
323342
}
324343

‎tests/unit/provider-share-command.test.mjs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,87 @@ test('loadAll can refresh in background without flipping the global loading stat
757757
assert.strictEqual(context.initError, '');
758758
});
759759

760+
test('loadAll falls back to medium for unsupported reasoning effort values while preserving xhigh', async () => {
761+
const loadAllSource = extractBlockBySignature(
762+
appSource,
763+
'async loadAll(options = {}) {'
764+
).replace(/^async loadAll/, 'async function loadAll');
765+
const responses = [
766+
{
767+
provider: 'alpha',
768+
model: 'alpha-model',
769+
serviceTier: 'fast',
770+
modelReasoningEffort: 'bogus',
771+
modelContextWindow: 200000,
772+
modelAutoCompactTokenLimit: 180000,
773+
configReady: true,
774+
initNotice: ''
775+
},
776+
{
777+
provider: 'alpha',
778+
model: 'alpha-model',
779+
serviceTier: 'fast',
780+
modelReasoningEffort: 'xhigh',
781+
modelContextWindow: 200000,
782+
modelAutoCompactTokenLimit: 180000,
783+
configReady: true,
784+
initNotice: ''
785+
}
786+
];
787+
let statusIndex = 0;
788+
const loadAll = instantiateFunction(loadAllSource, 'loadAll', {
789+
defaultModelContextWindow: 190000,
790+
defaultModelAutoCompactTokenLimit: 185000,
791+
api: async (action) => {
792+
if (action === 'status') {
793+
return responses[statusIndex++] || responses[responses.length - 1];
794+
}
795+
if (action === 'list') {
796+
return {
797+
providers: [{ name: 'alpha', url: 'https://api.example.com/v1', hasKey: true }]
798+
};
799+
}
800+
throw new Error(`Unexpected api action: ${action}`);
801+
}
802+
});
803+
804+
const createContext = () => ({
805+
loading: false,
806+
initError: '',
807+
currentProvider: 'stale-provider',
808+
currentModel: 'stale-model',
809+
serviceTier: 'fast',
810+
modelReasoningEffort: 'high',
811+
modelContextWindowInput: '190000',
812+
modelAutoCompactTokenLimitInput: '185000',
813+
editingCodexBudgetField: '',
814+
providersList: [],
815+
normalizePositiveIntegerInput(value, label, fallback = '') {
816+
const raw = value === undefined || value === null || value === ''
817+
? String(fallback || '')
818+
: String(value);
819+
const text = raw.trim();
820+
const numeric = Number.parseInt(text, 10);
821+
if (!Number.isFinite(numeric) || numeric <= 0) {
822+
return { ok: false, error: `${label} invalid` };
823+
}
824+
return { ok: true, value: numeric, text: String(numeric) };
825+
},
826+
showMessage() {},
827+
maybeShowStarPrompt() {},
828+
async loadModelsForProvider() {},
829+
async loadCodexAuthProfiles() {}
830+
});
831+
832+
const invalidContext = createContext();
833+
await loadAll.call(invalidContext);
834+
assert.strictEqual(invalidContext.modelReasoningEffort, 'medium');
835+
836+
const xhighContext = createContext();
837+
await loadAll.call(xhighContext);
838+
assert.strictEqual(xhighContext.modelReasoningEffort, 'xhigh');
839+
});
840+
760841
test('loadAll treats provider list fetch failures as startup errors and skips model refresh', async () => {
761842
const loadAllSource = extractBlockBySignature(
762843
appSource,

‎tests/unit/web-run-host.test.mjs‎

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ const resolveWebHostSource = extractFunctionBySignature(
118118
'function resolveWebHost(options = {}) {',
119119
'resolveWebHost'
120120
);
121+
const cmdStartSource = extractFunctionBySignature(
122+
cliContent,
123+
'function cmdStart(options = {}) {',
124+
'cmdStart'
125+
);
121126
const releaseRunPortIfNeededSource = extractFunctionBySignature(
122127
cliContent,
123128
'function releaseRunPortIfNeeded(port, deps = {}) {',
@@ -180,24 +185,45 @@ test('releaseRunPortIfNeeded skips non-default ports', () => {
180185
assert.deepStrictEqual(calls, []);
181186
});
182187

183-
test('releaseRunPortIfNeeded clears default port via fuser on linux', () => {
188+
test('releaseRunPortIfNeeded clears default port only after lsof pids map to managed run processes', () => {
184189
const calls = [];
190+
const killed = [];
185191
const logs = [];
186192
const releaseRunPortIfNeeded = instantiateFunction(releaseRunPortIfNeededSource, 'releaseRunPortIfNeeded', {
187193
DEFAULT_WEB_PORT: 3737,
188194
spawnSync(command, args) {
189195
calls.push([command, args]);
190-
if (command === 'fuser') {
191-
return { status: 0, stdout: '1234\n', stderr: '' };
196+
if (command === 'lsof') {
197+
return { status: 0, stdout: '1234\n8888\n', stderr: '' };
198+
}
199+
if (command === 'ps') {
200+
return {
201+
status: 0,
202+
stdout: [
203+
'UID PID PPID C STIME TTY TIME CMD',
204+
'u0_a876 1234 1000 0 1970 ? 00:00:00 node /repo/cli.js run --no-browser',
205+
'u0_a876 8888 1000 0 1970 ? 00:00:00 node /repo/other-server.js'
206+
].join('\n'),
207+
stderr: ''
208+
};
192209
}
193210
throw new Error(`unexpected command: ${command}`);
194211
},
195-
process: { platform: 'linux' },
212+
process: {
213+
platform: 'linux',
214+
kill(pid, signal) {
215+
killed.push([pid, signal]);
216+
}
217+
},
196218
console: { log(message) { logs.push(message); } }
197219
});
198220

199221
const result = releaseRunPortIfNeeded(3737);
200-
assert.deepStrictEqual(calls, [['fuser', ['-k', '3737/tcp']]]);
222+
assert.deepStrictEqual(calls, [
223+
['lsof', ['-ti', 'tcp:3737']],
224+
['ps', ['-ef']]
225+
]);
226+
assert.deepStrictEqual(killed, [[1234, 'SIGKILL']]);
201227
assert.deepStrictEqual(result, {
202228
attempted: true,
203229
released: true,
@@ -206,18 +232,26 @@ test('releaseRunPortIfNeeded clears default port via fuser on linux', () => {
206232
assert.deepStrictEqual(logs, ['~ 已释放端口 3737 占用']);
207233
});
208234

209-
test('releaseRunPortIfNeeded falls back to lsof pids when fuser is unavailable', () => {
235+
test('releaseRunPortIfNeeded falls back to ps scan when lsof is unavailable', () => {
210236
const calls = [];
211237
const killed = [];
212238
const releaseRunPortIfNeeded = instantiateFunction(releaseRunPortIfNeededSource, 'releaseRunPortIfNeeded', {
213239
DEFAULT_WEB_PORT: 3737,
214240
spawnSync(command, args) {
215241
calls.push([command, args]);
216-
if (command === 'fuser') {
242+
if (command === 'lsof') {
217243
return { error: { code: 'ENOENT' }, status: null, stdout: '', stderr: '' };
218244
}
219-
if (command === 'lsof') {
220-
return { status: 0, stdout: '2222\n3333\n', stderr: '' };
245+
if (command === 'ps') {
246+
return {
247+
status: 0,
248+
stdout: [
249+
'UID PID PPID C STIME TTY TIME CMD',
250+
'u0_a876 2222 1000 0 1970 ? 00:00:00 node /repo/cli.js run --no-browser',
251+
'u0_a876 3333 1000 0 1970 ? 00:00:00 /usr/bin/codexmate run'
252+
].join('\n'),
253+
stderr: ''
254+
};
221255
}
222256
throw new Error(`unexpected command: ${command}`);
223257
},
@@ -232,8 +266,8 @@ test('releaseRunPortIfNeeded falls back to lsof pids when fuser is unavailable',
232266

233267
const result = releaseRunPortIfNeeded(3737);
234268
assert.deepStrictEqual(calls, [
235-
['fuser', ['-k', '3737/tcp']],
236-
['lsof', ['-ti', 'tcp:3737']]
269+
['lsof', ['-ti', 'tcp:3737']],
270+
['ps', ['-ef']]
237271
]);
238272
assert.deepStrictEqual(killed, [
239273
[2222, 'SIGKILL'],
@@ -253,11 +287,8 @@ test('releaseRunPortIfNeeded falls back to ps scan for managed run processes', (
253287
DEFAULT_WEB_PORT: 3737,
254288
spawnSync(command, args) {
255289
calls.push([command, args]);
256-
if (command === 'fuser') {
257-
return { error: { code: 'EACCES' }, status: 1, stdout: '', stderr: 'Permission denied' };
258-
}
259290
if (command === 'lsof') {
260-
return { error: { code: 'ENOENT' }, status: null, stdout: '', stderr: '' };
291+
return { status: 0, stdout: '9001\n9002\n', stderr: '' };
261292
}
262293
if (command === 'ps') {
263294
return {
@@ -285,7 +316,6 @@ test('releaseRunPortIfNeeded falls back to ps scan for managed run processes', (
285316

286317
const result = releaseRunPortIfNeeded(3737);
287318
assert.deepStrictEqual(calls, [
288-
['fuser', ['-k', '3737/tcp']],
289319
['lsof', ['-ti', 'tcp:3737']],
290320
['ps', ['-ef']]
291321
]);
@@ -298,7 +328,15 @@ test('releaseRunPortIfNeeded falls back to ps scan for managed run processes', (
298328
});
299329

300330
test('cmdStart releases the resolved port before creating the web server', () => {
301-
assert.match(cliContent, /const port = resolveWebPort\(\);\s*const host = resolveWebHost\(options\);\s*releaseRunPortIfNeeded\(port\);\s*let serverHandle = createWebServer\(/s);
331+
const resolveIndex = cmdStartSource.indexOf('resolveWebPort(');
332+
const releaseIndex = cmdStartSource.indexOf('releaseRunPortIfNeeded(');
333+
const createIndex = cmdStartSource.indexOf('createWebServer(');
334+
335+
assert(resolveIndex >= 0, 'cmdStart should resolve the web port');
336+
assert(releaseIndex >= 0, 'cmdStart should release the run port before startup');
337+
assert(createIndex >= 0, 'cmdStart should create the web server');
338+
assert(resolveIndex < releaseIndex, 'cmdStart should resolve the port before releasing it');
339+
assert(releaseIndex < createIndex, 'cmdStart should release the port before creating the web server');
302340
});
303341

304342
const getCodexSkillsDirSource = extractFunctionBySignature(

‎web-ui/modules/app.methods.startup-claude.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ export function createStartupClaudeMethods(options = {}) {
4040
const effort = typeof statusRes.modelReasoningEffort === 'string'
4141
? statusRes.modelReasoningEffort.trim().toLowerCase()
4242
: '';
43-
this.modelReasoningEffort = effort || 'medium';
43+
const allowedReasoningEfforts = new Set(['low', 'medium', 'high', 'xhigh']);
44+
this.modelReasoningEffort = allowedReasoningEfforts.has(effort) ? effort : 'medium';
4445
}
4546
{
4647
const contextWindow = this.normalizePositiveIntegerInput(

0 commit comments

Comments
 (0)