Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 145 additions & 1 deletion cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,150 @@ function resolveWebPort() {
return parsed;
}

// #region releaseRunPortIfNeeded
function releaseRunPortIfNeeded(port, deps = {}) {
const numericPort = parseInt(String(port), 10);
if (numericPort !== DEFAULT_WEB_PORT) {
return { attempted: false, released: false, pids: [], reason: 'non-default-port' };
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

const processRef = deps.process || process;
const runSpawnSync = deps.spawnSync || spawnSync;
const logger = deps.logger || console;
const killProcess = typeof deps.kill === 'function'
? deps.kill
: (typeof processRef.kill === 'function' ? processRef.kill.bind(processRef) : null);
const seenPids = new Set();
const candidatePids = new Set();
const currentPid = Number(processRef.pid);
let released = false;

const addPidsFromText = (text, targetSet = seenPids) => {
if (!targetSet) {
return;
}
const lines = String(text || '').split(/\r?\n/);
for (const line of lines) {
const trimmed = line.trim();
if (!/^\d+$/.test(trimmed)) {
continue;
}
targetSet.add(Number(trimmed));
}
};

const runCommand = (command, args, options = {}) => {
const {
stdoutPidSet = seenPids,
stderrPidSet = seenPids
} = options;
const result = runSpawnSync(command, args, { encoding: 'utf8' });
if (result && result.stdout) addPidsFromText(result.stdout, stdoutPidSet);
if (result && result.stderr) addPidsFromText(result.stderr, stderrPidSet);
return result || {};
};

const addManagedRunPidsFromPs = (text, allowedPids = null) => {
const lines = String(text || '').split(/\r?\n/);
for (const line of lines) {
const normalizedLine = ` ${line.replace(/\s+/g, ' ').trim()} `;
if (!/(^|[\/\s])codexmate run(\s|$)/.test(normalizedLine) && !/(^|[\/\s])cli\.js run(\s|$)/.test(normalizedLine)) {
continue;
}
const pidMatch = line.match(/^\S+\s+(\d+)\s+/);
if (!pidMatch) {
continue;
}
const pid = Number(pidMatch[1]);
if (!Number.isFinite(pid) || pid <= 0 || pid === currentPid) {
continue;
}
if (allowedPids && !allowedPids.has(pid)) {
continue;
}
seenPids.add(pid);
}
};

if (processRef.platform === 'win32') {
const netstatResult = runCommand('netstat', ['-ano', '-p', 'tcp']);
if (!(netstatResult && netstatResult.error)) {
const lines = String(netstatResult.stdout || '').split(/\r?\n/);
for (const line of lines) {
const parts = line.trim().split(/\s+/);
if (parts.length < 5) {
continue;
}
const localAddress = parts[1];
const state = parts[3];
const pidText = parts[4];
if (state !== 'LISTENING' || !localAddress.endsWith(`:${numericPort}`) || !/^\d+$/.test(pidText)) {
continue;
}
seenPids.add(Number(pidText));
}
for (const pid of seenPids) {
const taskkillResult = runCommand('taskkill', ['/PID', String(pid), '/F']);
if (!taskkillResult.error && taskkillResult.status === 0) {
released = true;
}
}
}
} else {
let psResult = null;
const readPsResult = () => {
if (psResult) {
return psResult;
}
psResult = runCommand('ps', ['-ef'], { stdoutPidSet: null, stderrPidSet: null });
return psResult;
};

const lsofResult = runCommand(
'lsof',
['-ti', `tcp:${numericPort}`],
{ stdoutPidSet: candidatePids, stderrPidSet: null }
);
if (!(lsofResult && lsofResult.error) && candidatePids.size > 0) {
const managedPsResult = readPsResult();
if (!(managedPsResult && managedPsResult.error)) {
addManagedRunPidsFromPs(managedPsResult.stdout, candidatePids);
}
}
if (killProcess && seenPids.size === 0) {
const managedPsResult = readPsResult();
if (!(managedPsResult && managedPsResult.error)) {
addManagedRunPidsFromPs(managedPsResult.stdout);
}
}
}

if (processRef.platform !== 'win32' && killProcess && !released && seenPids.size > 0) {
for (const pid of seenPids) {
if (pid === currentPid) {
continue;
}
try {
killProcess(pid, 'SIGKILL');
released = true;
Comment on lines +412 to +440
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unix cleanup still kills by port instead of by requested listener.

The non-Windows path ignores host entirely: lsof/fuser collect every managed codexmate run on port 3737, then Line 433 SIGKILLs all verified matches. That can terminate another instance bound to a different specific address even when the requested --host would coexist. Filter Unix candidates by local listener address before they reach seenPids, or skip cleanup when the listener address cannot be verified.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli.js` around lines 412 - 440, The cleanup code uses lsof/fuser and
candidatePids/seenPids to kill by port only, ignoring the requested host, so
killProcess(currentPid) may terminate other instances bound to different
addresses; update the non-Windows path (the runCommand/lsof/fuser logic that
populates candidatePids, the call sites readPsResult and
addManagedRunPidsFromPs, and the loop that iterates seenPids before calling
killProcess) to verify each candidate's local listener address matches the
requested host (or skip if verification is impossible) before adding to seenPids
or issuing SIGKILL; i.e., after obtaining PIDs from lsof/fuser/readPsResult,
query the listener address for each PID (or use lsof output flags to include
address) and filter candidatePids so only processes bound to the same host:port
are left, otherwise do not perform cleanup.

} catch (_) {}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

if (released) {
logger.log(`~ 已释放端口 ${numericPort} 占用`);
}

return {
attempted: true,
released,
pids: Array.from(seenPids)
.filter((pid) => pid !== currentPid)
.sort((a, b) => a - b)
};
}
// #endregion releaseRunPortIfNeeded

function resolveWebHost(options = {}) {
const optionHost = typeof options.host === 'string' ? options.host.trim() : '';
if (optionHost) {
Expand All @@ -236,7 +380,6 @@ function resolveWebHost(options = {}) {
}

const EMPTY_CONFIG_FALLBACK_TEMPLATE = `model = "gpt-5.3-codex"
model_reasoning_effort = "high"
model_context_window = ${DEFAULT_MODEL_CONTEXT_WINDOW}
model_auto_compact_token_limit = ${DEFAULT_MODEL_AUTO_COMPACT_TOKEN_LIMIT}
disable_response_storage = true
Expand Down Expand Up @@ -10845,6 +10988,7 @@ function cmdStart(options = {}) {

const port = resolveWebPort();
const host = resolveWebHost(options);
releaseRunPortIfNeeded(port);

let serverHandle = createWebServer({
htmlPath,
Expand Down
4 changes: 4 additions & 0 deletions tests/e2e/test-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,10 @@ preferred_auth_method = "shadow-key"
/^\s*model_auto_compact_token_limit\s*=\s*185000\s*$/m.test(legacyTemplateDefaults.template),
'legacy get-config-template should restore default model_auto_compact_token_limit'
);
assert(
!/^\s*model_reasoning_effort\s*=.+$/m.test(legacyTemplateDefaults.template),
'legacy get-config-template should keep default medium reasoning without model_reasoning_effort'
);
const legacyAddDup = await legacyApi('add-provider', {
name: 'foo.bar',
url: 'https://dup.example.com/v1',
Expand Down
81 changes: 81 additions & 0 deletions tests/unit/provider-share-command.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,87 @@ test('loadAll can refresh in background without flipping the global loading stat
assert.strictEqual(context.initError, '');
});

test('loadAll falls back to medium for unsupported reasoning effort values while preserving xhigh', async () => {
const loadAllSource = extractBlockBySignature(
appSource,
'async loadAll(options = {}) {'
).replace(/^async loadAll/, 'async function loadAll');
const responses = [
{
provider: 'alpha',
model: 'alpha-model',
serviceTier: 'fast',
modelReasoningEffort: 'bogus',
modelContextWindow: 200000,
modelAutoCompactTokenLimit: 180000,
configReady: true,
initNotice: ''
},
{
provider: 'alpha',
model: 'alpha-model',
serviceTier: 'fast',
modelReasoningEffort: 'xhigh',
modelContextWindow: 200000,
modelAutoCompactTokenLimit: 180000,
configReady: true,
initNotice: ''
}
];
let statusIndex = 0;
const loadAll = instantiateFunction(loadAllSource, 'loadAll', {
defaultModelContextWindow: 190000,
defaultModelAutoCompactTokenLimit: 185000,
api: async (action) => {
if (action === 'status') {
return responses[statusIndex++] || responses[responses.length - 1];
}
if (action === 'list') {
return {
providers: [{ name: 'alpha', url: 'https://api.example.com/v1', hasKey: true }]
};
}
throw new Error(`Unexpected api action: ${action}`);
}
});

const createContext = () => ({
loading: false,
initError: '',
currentProvider: 'stale-provider',
currentModel: 'stale-model',
serviceTier: 'fast',
modelReasoningEffort: 'high',
modelContextWindowInput: '190000',
modelAutoCompactTokenLimitInput: '185000',
editingCodexBudgetField: '',
providersList: [],
normalizePositiveIntegerInput(value, label, fallback = '') {
const raw = value === undefined || value === null || value === ''
? String(fallback || '')
: String(value);
const text = raw.trim();
const numeric = Number.parseInt(text, 10);
if (!Number.isFinite(numeric) || numeric <= 0) {
return { ok: false, error: `${label} invalid` };
}
return { ok: true, value: numeric, text: String(numeric) };
},
showMessage() {},
maybeShowStarPrompt() {},
async loadModelsForProvider() {},
async loadCodexAuthProfiles() {}
});

const invalidContext = createContext();
await loadAll.call(invalidContext);
assert.strictEqual(invalidContext.modelReasoningEffort, 'medium');

const xhighContext = createContext();
await loadAll.call(xhighContext);
assert.strictEqual(xhighContext.modelReasoningEffort, 'xhigh');
});

test('loadAll treats provider list fetch failures as startup errors and skips model refresh', async () => {
const loadAllSource = extractBlockBySignature(
appSource,
Expand Down
Loading
Loading