feat: add pnpm store mount, pre-flight checks, and idle timeout for reviewer containers
- Detect pnpm projects and mount global store read-only so hardlinks resolve - npm/yarn/bun need no extra mounts (node_modules is self-contained) - Pre-flight check: verify Docker running + image exists (cached after first call) - Add activity-based idle timeout (reset on stdout/stderr like host runner) - Clean up orphaned containers on shutdown (not just startup)
This commit is contained in:
@@ -7,8 +7,9 @@
|
|||||||
* - IPC via filesystem (input/ directory for follow-up messages)
|
* - IPC via filesystem (input/ directory for follow-up messages)
|
||||||
* - Credentials injected by the credential proxy (never exposed to container)
|
* - Credentials injected by the credential proxy (never exposed to container)
|
||||||
*/
|
*/
|
||||||
import { ChildProcess, spawn } from 'child_process';
|
import { ChildProcess, execFileSync, spawn } from 'child_process';
|
||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
|
import os from 'os';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
|
|
||||||
import { OUTPUT_END_MARKER, OUTPUT_START_MARKER } from './agent-protocol.js';
|
import { OUTPUT_END_MARKER, OUTPUT_START_MARKER } from './agent-protocol.js';
|
||||||
@@ -23,6 +24,7 @@ import {
|
|||||||
import {
|
import {
|
||||||
CONTAINER_HOST_GATEWAY,
|
CONTAINER_HOST_GATEWAY,
|
||||||
CONTAINER_RUNTIME_BIN,
|
CONTAINER_RUNTIME_BIN,
|
||||||
|
ensureContainerRuntimeRunning,
|
||||||
hostGatewayArgs,
|
hostGatewayArgs,
|
||||||
readonlyMountArgs,
|
readonlyMountArgs,
|
||||||
stopContainer,
|
stopContainer,
|
||||||
@@ -62,6 +64,55 @@ interface VolumeMount {
|
|||||||
readonly: boolean;
|
readonly: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── pnpm store detection ─────────────────────────────────────────
|
||||||
|
|
||||||
|
function detectPnpmStorePath(workspaceDir: string): string | null {
|
||||||
|
if (!fs.existsSync(path.join(workspaceDir, 'pnpm-lock.yaml'))) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
// Check env override first
|
||||||
|
if (process.env.PNPM_STORE_DIR && fs.existsSync(process.env.PNPM_STORE_DIR)) {
|
||||||
|
return process.env.PNPM_STORE_DIR;
|
||||||
|
}
|
||||||
|
// Try `pnpm store path`
|
||||||
|
try {
|
||||||
|
const storePath = execFileSync('pnpm', ['store', 'path'], {
|
||||||
|
cwd: workspaceDir,
|
||||||
|
encoding: 'utf-8',
|
||||||
|
stdio: ['ignore', 'pipe', 'pipe'],
|
||||||
|
timeout: 5000,
|
||||||
|
}).trim();
|
||||||
|
if (storePath && fs.existsSync(storePath)) return storePath;
|
||||||
|
} catch {
|
||||||
|
/* pnpm not available */
|
||||||
|
}
|
||||||
|
// Fallback to default location
|
||||||
|
const defaultStore = path.join(os.homedir(), '.local', 'share', 'pnpm', 'store');
|
||||||
|
if (fs.existsSync(defaultStore)) return defaultStore;
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Pre-flight checks ────────────────────────────────────────────
|
||||||
|
|
||||||
|
let containerRuntimeChecked = false;
|
||||||
|
|
||||||
|
function ensureContainerReady(): void {
|
||||||
|
if (containerRuntimeChecked) return;
|
||||||
|
ensureContainerRuntimeRunning();
|
||||||
|
// Check image exists
|
||||||
|
try {
|
||||||
|
execFileSync(CONTAINER_RUNTIME_BIN, ['image', 'inspect', CONTAINER_IMAGE], {
|
||||||
|
stdio: 'pipe',
|
||||||
|
timeout: 10000,
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
throw new Error(
|
||||||
|
`Container image '${CONTAINER_IMAGE}' not found. Build it with: ./container/build.sh`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
containerRuntimeChecked = true;
|
||||||
|
}
|
||||||
|
|
||||||
// ── Mount builder ─────────────────────────────────────────────────
|
// ── Mount builder ─────────────────────────────────────────────────
|
||||||
|
|
||||||
export function buildReviewerMounts(
|
export function buildReviewerMounts(
|
||||||
@@ -73,12 +124,25 @@ export function buildReviewerMounts(
|
|||||||
const groupIpcDir = resolveGroupIpcPath(group.folder);
|
const groupIpcDir = resolveGroupIpcPath(group.folder);
|
||||||
|
|
||||||
// Source code: READ-ONLY (kernel-level protection)
|
// Source code: READ-ONLY (kernel-level protection)
|
||||||
|
// Includes node_modules if present (npm/yarn/bun are self-contained)
|
||||||
mounts.push({
|
mounts.push({
|
||||||
hostPath: ownerWorkspaceDir,
|
hostPath: ownerWorkspaceDir,
|
||||||
containerPath: '/workspace/project',
|
containerPath: '/workspace/project',
|
||||||
readonly: true,
|
readonly: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// pnpm global store: mount at the same host path so hardlinks resolve.
|
||||||
|
// Only needed for pnpm — npm/yarn/bun have self-contained node_modules.
|
||||||
|
const pnpmStore = detectPnpmStorePath(ownerWorkspaceDir);
|
||||||
|
if (pnpmStore) {
|
||||||
|
mounts.push({
|
||||||
|
hostPath: pnpmStore,
|
||||||
|
containerPath: pnpmStore,
|
||||||
|
readonly: true,
|
||||||
|
});
|
||||||
|
logger.debug({ pnpmStore }, 'Mounting pnpm store for container');
|
||||||
|
}
|
||||||
|
|
||||||
// Shadow .env so reviewer cannot read secrets from mounted project
|
// Shadow .env so reviewer cannot read secrets from mounted project
|
||||||
const envFile = path.join(ownerWorkspaceDir, '.env');
|
const envFile = path.join(ownerWorkspaceDir, '.env');
|
||||||
if (fs.existsSync(envFile)) {
|
if (fs.existsSync(envFile)) {
|
||||||
@@ -215,6 +279,9 @@ export async function runReviewerContainer(args: {
|
|||||||
const startTime = Date.now();
|
const startTime = Date.now();
|
||||||
const containerName = `ejclaw-reviewer-${group.folder}-${Date.now()}`;
|
const containerName = `ejclaw-reviewer-${group.folder}-${Date.now()}`;
|
||||||
|
|
||||||
|
// Pre-flight: Docker running + image exists (cached after first check)
|
||||||
|
ensureContainerReady();
|
||||||
|
|
||||||
const mounts = buildReviewerMounts(group, ownerWorkspaceDir);
|
const mounts = buildReviewerMounts(group, ownerWorkspaceDir);
|
||||||
const containerArgs = buildContainerArgs(mounts, containerName, envOverrides);
|
const containerArgs = buildContainerArgs(mounts, containerName, envOverrides);
|
||||||
|
|
||||||
@@ -269,6 +336,7 @@ export async function runReviewerContainer(args: {
|
|||||||
|
|
||||||
stdout += text;
|
stdout += text;
|
||||||
parseBuffer += text;
|
parseBuffer += text;
|
||||||
|
resetTimeout();
|
||||||
|
|
||||||
// Parse streamed output markers
|
// Parse streamed output markers
|
||||||
let startIdx: number;
|
let startIdx: number;
|
||||||
@@ -296,12 +364,15 @@ export async function runReviewerContainer(args: {
|
|||||||
|
|
||||||
proc.stderr.on('data', (chunk: Buffer) => {
|
proc.stderr.on('data', (chunk: Buffer) => {
|
||||||
stderr += chunk.toString();
|
stderr += chunk.toString();
|
||||||
|
// Stderr activity means agent is alive — reset idle timeout
|
||||||
|
resetTimeout();
|
||||||
});
|
});
|
||||||
|
|
||||||
// Timeout
|
// Activity-based timeout: reset on stdout/stderr data
|
||||||
const timeout = setTimeout(() => {
|
const timeoutMs = Math.max(AGENT_TIMEOUT, IDLE_TIMEOUT + 30_000);
|
||||||
|
const killOnTimeout = () => {
|
||||||
logger.warn(
|
logger.warn(
|
||||||
{ containerName, timeoutMs: AGENT_TIMEOUT },
|
{ containerName, timeoutMs },
|
||||||
'Reviewer container timed out, stopping',
|
'Reviewer container timed out, stopping',
|
||||||
);
|
);
|
||||||
try {
|
try {
|
||||||
@@ -309,7 +380,12 @@ export async function runReviewerContainer(args: {
|
|||||||
} catch {
|
} catch {
|
||||||
/* ignore */
|
/* ignore */
|
||||||
}
|
}
|
||||||
}, AGENT_TIMEOUT);
|
};
|
||||||
|
let timeout = setTimeout(killOnTimeout, timeoutMs);
|
||||||
|
const resetTimeout = () => {
|
||||||
|
clearTimeout(timeout);
|
||||||
|
timeout = setTimeout(killOnTimeout, timeoutMs);
|
||||||
|
};
|
||||||
|
|
||||||
proc.on('close', (code, signal) => {
|
proc.on('close', (code, signal) => {
|
||||||
clearTimeout(timeout);
|
clearTimeout(timeout);
|
||||||
|
|||||||
@@ -377,6 +377,7 @@ async function main(): Promise<void> {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
await queue.shutdown(10000);
|
await queue.shutdown(10000);
|
||||||
|
cleanupOrphans();
|
||||||
for (const ch of channels) await ch.disconnect();
|
for (const ch of channels) await ch.disconnect();
|
||||||
process.exit(0);
|
process.exit(0);
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user