fix: use etime-based sorting instead of PID ordering for process guards
Addresses Greptile review feedback: - ChromaSync: replace PID-based sort with ps etime column + parseElapsedTime() for reliable age ordering (PIDs wrap and don't guarantee ordering) - ProcessManager: filter out entries with unparseable etime (-1) before sorting to prevent sort corruption in cleanupExcessChromaProcesses()
This commit is contained in:
@@ -369,6 +369,8 @@ export async function cleanupExcessChromaProcesses(maxAllowed: number = 2): Prom
|
|||||||
if (!Number.isInteger(pid) || pid <= 0 || pid === process.pid) continue;
|
if (!Number.isInteger(pid) || pid <= 0 || pid === process.pid) continue;
|
||||||
|
|
||||||
const ageMinutes = parseElapsedTime(etime);
|
const ageMinutes = parseElapsedTime(etime);
|
||||||
|
// Skip entries with unparseable etime (-1) to avoid sort corruption
|
||||||
|
if (ageMinutes < 0) continue;
|
||||||
processes.push({ pid, ageMinutes });
|
processes.push({ pid, ageMinutes });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import path from 'path';
|
|||||||
import os from 'os';
|
import os from 'os';
|
||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
import { execSync, execFileSync } from 'child_process';
|
import { execSync, execFileSync } from 'child_process';
|
||||||
|
import { parseElapsedTime } from '../infrastructure/ProcessManager.js';
|
||||||
|
|
||||||
// Version injected at build time by esbuild define
|
// Version injected at build time by esbuild define
|
||||||
declare const __DEFAULT_PACKAGE_VERSION__: string;
|
declare const __DEFAULT_PACKAGE_VERSION__: string;
|
||||||
@@ -242,25 +243,32 @@ export class ChromaSync {
|
|||||||
if (process.platform === 'win32') return; // Windows has Chroma disabled entirely
|
if (process.platform === 'win32') return; // Windows has Chroma disabled entirely
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Use execFileSync to avoid shell injection — filter in JavaScript
|
// Use execFileSync to avoid shell injection — filter and sort in JavaScript
|
||||||
const output = execFileSync('ps', ['-eo', 'pid,command'], {
|
// Include etime column for reliable age-based sorting (PID order is unreliable)
|
||||||
|
const output = execFileSync('ps', ['-eo', 'pid,etime,command'], {
|
||||||
encoding: 'utf8',
|
encoding: 'utf8',
|
||||||
timeout: 5000,
|
timeout: 5000,
|
||||||
stdio: ['pipe', 'pipe', 'pipe']
|
stdio: ['pipe', 'pipe', 'pipe']
|
||||||
});
|
});
|
||||||
|
|
||||||
// Filter for chroma-mcp processes in JavaScript (no shell grep needed)
|
// Filter for chroma-mcp, parse elapsed time, sort by actual age
|
||||||
const pids = output.split('\n')
|
const processes = output.split('\n')
|
||||||
.filter(l => l.includes('chroma-mcp'))
|
.filter(l => l.includes('chroma-mcp'))
|
||||||
.map(l => parseInt(l.trim().split(/\s+/)[0], 10))
|
.map(l => {
|
||||||
.filter(pid => pid > 0 && pid !== process.pid)
|
const parts = l.trim().split(/\s+/);
|
||||||
.sort((a, b) => b - a); // Descending: newest (highest PID) first
|
const pid = parseInt(parts[0], 10);
|
||||||
|
const etime = parts[1] || '';
|
||||||
|
const ageMinutes = parseElapsedTime(etime);
|
||||||
|
return { pid, ageMinutes };
|
||||||
|
})
|
||||||
|
.filter(p => p.pid > 0 && p.pid !== process.pid && p.ageMinutes >= 0)
|
||||||
|
.sort((a, b) => a.ageMinutes - b.ageMinutes); // Ascending: newest (lowest age) first
|
||||||
|
|
||||||
if (pids.length < MAX_CHROMA_PROCESSES) return;
|
if (processes.length < MAX_CHROMA_PROCESSES) return;
|
||||||
|
|
||||||
// Keep newest MAX_CHROMA_PROCESSES - 1 (making room for the one we're about to spawn)
|
// Keep newest MAX_CHROMA_PROCESSES - 1 (making room for the one we're about to spawn)
|
||||||
const toKill = pids.slice(MAX_CHROMA_PROCESSES - 1);
|
const toKill = processes.slice(MAX_CHROMA_PROCESSES - 1);
|
||||||
for (const pid of toKill) {
|
for (const { pid } of toKill) {
|
||||||
try {
|
try {
|
||||||
process.kill(pid, 'SIGTERM');
|
process.kill(pid, 'SIGTERM');
|
||||||
} catch {
|
} catch {
|
||||||
@@ -270,7 +278,7 @@ export class ChromaSync {
|
|||||||
|
|
||||||
if (toKill.length > 0) {
|
if (toKill.length > 0) {
|
||||||
logger.warn('CHROMA_SYNC', 'Killed excess chroma-mcp processes before spawning', {
|
logger.warn('CHROMA_SYNC', 'Killed excess chroma-mcp processes before spawning', {
|
||||||
found: pids.length,
|
found: processes.length,
|
||||||
killed: toKill.length,
|
killed: toKill.length,
|
||||||
maxAllowed: MAX_CHROMA_PROCESSES
|
maxAllowed: MAX_CHROMA_PROCESSES
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user