Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
235 changes: 234 additions & 1 deletion cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,239 @@ function resolveWebPort() {
return parsed;
}

// #region releaseRunPortIfNeeded
function releaseRunPortIfNeeded(port, host, deps = {}) {
const numericPort = parseInt(String(port), 10);
if (numericPort !== DEFAULT_WEB_PORT) {
return { attempted: false, released: false, pids: [], reason: 'non-default-port' };
}

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);
const normalizedHost = typeof host === 'string' ? host.trim().toLowerCase() : '';
let released = false;
const windowsCommandLineCache = new Map();

const isManagedRunCommand = (commandLine) => {
const normalizedLine = ` ${String(commandLine || '').replace(/\s+/g, ' ').trim()} `;
return /(^|[\/\\\s])codexmate(?:\.cmd|\.exe)? run(\s|$)/i.test(normalizedLine)
|| /(^|[\/\\\s])cli\.js run(\s|$)/i.test(normalizedLine);
};

const normalizeListenerHost = (value) => {
const trimmed = String(value || '').trim().toLowerCase();
if (!trimmed) {
return '';
}
if (trimmed.startsWith('[') && trimmed.endsWith(']')) {
return trimmed.slice(1, -1);
}
return trimmed.startsWith('::ffff:') ? trimmed.slice('::ffff:'.length) : trimmed;
};

const extractListenerHost = (localAddress) => {
const trimmed = String(localAddress || '').trim();
if (!trimmed) {
return '';
}
if (trimmed.startsWith('[')) {
const closingBracket = trimmed.indexOf(']');
if (closingBracket > 0) {
return normalizeListenerHost(trimmed.slice(1, closingBracket));
}
}
const lastColon = trimmed.lastIndexOf(':');
if (lastColon <= 0) {
return normalizeListenerHost(trimmed);
}
return normalizeListenerHost(trimmed.slice(0, lastColon));
};

const isMatchingWindowsListenerAddress = (localAddress) => {
const listenerHost = extractListenerHost(localAddress);
if (!listenerHost || !normalizedHost) {
return false;
}
if (normalizedHost === 'localhost') {
return listenerHost === '127.0.0.1' || listenerHost === '::1';
}
if (normalizedHost === '0.0.0.0' || normalizedHost === '::') {
return listenerHost === normalizedHost;
}
return listenerHost === normalizeListenerHost(normalizedHost);
};
Comment on lines +281 to +293
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

Match wildcard listeners against host-specific Windows starts.

This matcher never treats 0.0.0.0:3737 / [::]:3737 listeners as conflicts for host-specific starts. A stale wildcard codexmate run will therefore survive a later --host localhost, --host 127.0.0.1, or other concrete-address start even though it still owns the port, so the cleanup can still fall through to EADDRINUSE.

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

In `@cli.js` around lines 281 - 293, The isMatchingWindowsListenerAddress matcher
currently ignores wildcard listeners (listenerHost '0.0.0.0' or '::') when
comparing against a concrete normalizedHost, allowing a wildcard listener like
'0.0.0.0:3737' to be treated as non-conflicting with '--host localhost' or
'--host 127.0.0.1'; update isMatchingWindowsListenerAddress to treat wildcard
listener hosts as matching any concrete normalizedHost (and still handle the
special 'localhost'↔'127.0.0.1'/'::1' mapping), i.e. after extracting
listenerHost and ensuring normalizedHost exists, return true if listenerHost is
'0.0.0.0' or '::' (or if normalizedHost is those values keep existing behavior),
otherwise preserve the existing localhost and normalized comparisons using
extractListenerHost, normalizedHost and normalizeListenerHost.


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

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);
}
};

const getWindowsProcessCommandLine = (pid) => {
if (windowsCommandLineCache.has(pid)) {
return windowsCommandLineCache.get(pid);
}
const result = runCommand(
'powershell',
[
'-NoProfile',
'-Command',
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($p) { $p.CommandLine }`
],
{ stdoutPidSet: null, stderrPidSet: null }
);
const commandLine = !result.error && result.status === 0
? String(result.stdout || '').trim()
: '';
windowsCommandLineCache.set(pid, commandLine);
return commandLine;
};

if (processRef.platform === 'win32') {
const netstatResult = runCommand('netstat', ['-ano', '-p', 'tcp'], { stdoutPidSet: null, stderrPidSet: null });
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;
}
if (!isMatchingWindowsListenerAddress(localAddress)) {
continue;
}
candidatePids.add(Number(pidText));
}
for (const pid of candidatePids) {
if (pid === currentPid) {
continue;
}
if (!isManagedRunCommand(getWindowsProcessCommandLine(pid))) {
continue;
}
seenPids.add(pid);
const taskkillResult = runCommand(
'taskkill',
['/PID', String(pid), '/F'],
{ stdoutPidSet: null, stderrPidSet: null }
);
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 }
);
const shouldTryFuser = !!(lsofResult && lsofResult.error && lsofResult.error.code === 'ENOENT');
if (shouldTryFuser && candidatePids.size === 0) {
runCommand(
'fuser',
[`${numericPort}/tcp`],
{ stdoutPidSet: candidatePids, stderrPidSet: candidatePids }
);
}
if (candidatePids.size > 0) {
const managedPsResult = readPsResult();
if (!(managedPsResult && managedPsResult.error)) {
addManagedRunPidsFromPs(managedPsResult.stdout, candidatePids);
}
}
}

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 (_) {}
}
}

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 +469,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 +11077,7 @@ function cmdStart(options = {}) {

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

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