Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
22 changes: 22 additions & 0 deletions jest-resolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Custom Jest resolver that handles ESM-only packages.
*
* Some packages (e.g., unicorn-magic, is-unicode-supported) only provide
* "import" conditions in their package.json exports maps. When babel-jest
* transforms ESM to CJS, the resulting require() calls fail because Jest's
* default resolver only checks "require" and "node" conditions.
*
* This resolver falls back to the "import" condition when the default
* resolution fails, allowing these packages to be resolved correctly.
*/
module.exports = (path, options) => {
try {
return options.defaultResolver(path, options);
} catch (error) {
// If default resolution fails, retry with "import" condition added
return options.defaultResolver(path, {
...options,
conditions: [...(options.conditions || []), 'import'],
});
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

jest-resolver.js retries resolution with an added "import" condition for any resolver error. This can mask unrelated resolution problems (typos, missing files, etc.) and make failures harder to diagnose. Consider only retrying for export-condition related errors (e.g., ERR_PACKAGE_PATH_NOT_EXPORTED / exports resolution failures) and rethrowing the original error when the fallback also fails.

Suggested change
module.exports = (path, options) => {
try {
return options.defaultResolver(path, options);
} catch (error) {
// If default resolution fails, retry with "import" condition added
return options.defaultResolver(path, {
...options,
conditions: [...(options.conditions || []), 'import'],
});
const isExportsResolutionError = (error) => {
if (!error) {
return false;
}
if (error.code === 'ERR_PACKAGE_PATH_NOT_EXPORTED') {
return true;
}
const message = typeof error.message === 'string' ? error.message : '';
return (
message.includes('Package subpath') ||
message.includes('package exports') ||
message.includes('conditional exports') ||
message.includes('No "exports" main defined')
);
};
module.exports = (path, options) => {
try {
return options.defaultResolver(path, options);
} catch (error) {
if (!isExportsResolutionError(error)) {
throw error;
}
try {
return options.defaultResolver(path, {
...options,
conditions: [...(options.conditions || []), 'import'],
});
} catch (_fallbackError) {
throw error;
}

Copilot uses AI. Check for mistakes.
}
};
6 changes: 4 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ module.exports = {
'^.+\\.js$': 'babel-jest',
},
transformIgnorePatterns: [
// Transform ESM-only packages (chalk, execa, commander, etc.)
'node_modules/(?!(chalk|execa|commander)/)',
// Transform ESM-only packages (chalk, execa, commander, and their transitive deps)
'node_modules/(?!(chalk|execa|commander|@sindresorhus/merge-streams|@sec-ant/readable-stream|figures|get-stream|human-signals|is-plain-obj|is-stream|is-unicode-supported|npm-run-path|parse-ms|path-key|pretty-ms|strip-final-newline|unicorn-magic|yoctocolors)/)',
],
// Custom resolver to handle ESM-only packages with exports maps that lack "require" conditions
resolver: '<rootDir>/jest-resolver.js',
coverageDirectory: 'coverage',
coverageReporters: ['text', 'lcov', 'html', 'json-summary'],
coverageThreshold: {
Expand Down
1,238 changes: 901 additions & 337 deletions package-lock.json

Large diffs are not rendered by default.

26 changes: 13 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,36 @@
"author": "GitHub",
"license": "MIT",
"dependencies": {
"chalk": "^4.1.2",
"commander": "^12.0.0",
"execa": "^5.1.1",
"chalk": "^5.6.2",
"commander": "^14.0.3",
"execa": "^9.6.1",
"js-yaml": "^4.1.1"
Comment on lines 46 to 50
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Bumping commander to v14 and execa to v9 increases the minimum Node requirements (commander v14 requires Node >=20; execa v9 is ESM-only). This repo’s release pipeline uses pkg (see package.json pkg.targets) which currently targets Node 18; that binary build/runtime will likely break after these bumps. Align the pkg targets (and any CI/release workflow expectations) with the new Node/runtime requirements.

Copilot uses AI. Check for mistakes.
},
"devDependencies": {
"@babel/core": "^7.29.0",
"@babel/preset-env": "^7.29.0",
"@commitlint/cli": "^20.4.1",
"@commitlint/config-conventional": "^20.4.1",
"@eslint/compat": "^2.0.0",
"@eslint/compat": "^2.0.4",
"@eslint/js": "^10.0.0",
"@types/glob": "^9.0.0",
"@types/jest": "^30.0.0",
"@types/js-yaml": "^4.0.5",
"@types/node": "^25.2.3",
"@typescript-eslint/eslint-plugin": "^8.55.0",
"@typescript-eslint/parser": "^8.55.0",
"esbuild": "^0.25.0",
"@types/node": "^25.5.2",
"@typescript-eslint/eslint-plugin": "^8.58.0",
"@typescript-eslint/parser": "^8.58.0",
"esbuild": "^0.28.0",
"babel-jest": "^30.2.0",
"eslint": "^10.0.0",
"eslint-plugin-security": "^3.0.1",
"eslint-plugin-security": "^4.0.0",
"glob": "^13.0.1",
"globals": "^17.0.0",
"husky": "^9.1.7",
"jest": "^30.2.0",
"markdownlint-cli2": "^0.21.0",
"ts-jest": "^29.4.6",
"typescript": "^5.0.0",
"typescript-eslint": "^8.0.0"
"markdownlint-cli2": "^0.22.0",
"ts-jest": "^29.4.9",
"typescript": "^6.0.2",
"typescript-eslint": "^8.58.0"
},
"overrides": {
"test-exclude": "^7.0.1",
Expand Down
41 changes: 19 additions & 22 deletions src/commands/predownload.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import { resolveImages, predownloadCommand, PredownloadOptions } from './predownload';

// Mock execa
jest.mock('execa', () => {
const mockExeca = jest.fn().mockResolvedValue({ stdout: '', stderr: '' });
return { __esModule: true, default: mockExeca };
});

// eslint-disable-next-line @typescript-eslint/no-require-imports
const execa = require('execa').default as jest.Mock;
// Mock execa (v9: named exports)
const mockExeca = jest.fn().mockResolvedValue({ stdout: '', stderr: '' });
jest.mock('execa', () => ({
execa: (...args: any[]) => mockExeca(...args),
}));

describe('predownload', () => {
describe('resolveImages', () => {
Expand Down Expand Up @@ -116,20 +113,20 @@ describe('predownload', () => {
};

beforeEach(() => {
execa.mockReset();
execa.mockResolvedValue({ stdout: '', stderr: '' });
mockExeca.mockReset();
mockExeca.mockResolvedValue({ stdout: '', stderr: '' });
});

it('should pull all resolved images', async () => {
await predownloadCommand(defaults);

expect(execa).toHaveBeenCalledTimes(2);
expect(execa).toHaveBeenCalledWith(
expect(mockExeca).toHaveBeenCalledTimes(2);
expect(mockExeca).toHaveBeenCalledWith(
'docker',
['pull', 'ghcr.io/github/gh-aw-firewall/squid:latest'],
{ stdio: 'inherit' },
);
expect(execa).toHaveBeenCalledWith(
expect(mockExeca).toHaveBeenCalledWith(
'docker',
['pull', 'ghcr.io/github/gh-aw-firewall/agent:latest'],
{ stdio: 'inherit' },
Expand All @@ -139,8 +136,8 @@ describe('predownload', () => {
it('should pull api-proxy when enabled', async () => {
await predownloadCommand({ ...defaults, enableApiProxy: true });

expect(execa).toHaveBeenCalledTimes(3);
expect(execa).toHaveBeenCalledWith(
expect(mockExeca).toHaveBeenCalledTimes(3);
expect(mockExeca).toHaveBeenCalledWith(
'docker',
['pull', 'ghcr.io/github/gh-aw-firewall/api-proxy:latest'],
{ stdio: 'inherit' },
Expand All @@ -150,21 +147,21 @@ describe('predownload', () => {
it('should pull cli-proxy and mcpg when enabled', async () => {
await predownloadCommand({ ...defaults, enableCliProxy: true });

expect(execa).toHaveBeenCalledTimes(4);
expect(execa).toHaveBeenCalledWith(
expect(mockExeca).toHaveBeenCalledTimes(4);
expect(mockExeca).toHaveBeenCalledWith(
'docker',
['pull', 'ghcr.io/github/gh-aw-firewall/cli-proxy:latest'],
{ stdio: 'inherit' },
);
expect(execa).toHaveBeenCalledWith(
expect(mockExeca).toHaveBeenCalledWith(
'docker',
['pull', 'ghcr.io/github/gh-aw-mcpg:v0.2.15'],
{ stdio: 'inherit' },
);
});

it('should throw with exitCode 1 when a pull fails', async () => {
execa
mockExeca
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockRejectedValueOnce(new Error('pull failed'));

Expand All @@ -178,18 +175,18 @@ describe('predownload', () => {
});

it('should continue pulling remaining images after a failure', async () => {
execa.mockRejectedValueOnce(new Error('pull failed')).mockResolvedValueOnce({ stdout: '', stderr: '' });
mockExeca.mockRejectedValueOnce(new Error('pull failed')).mockResolvedValueOnce({ stdout: '', stderr: '' });

await expect(predownloadCommand(defaults)).rejects.toThrow(
'1 of 2 image(s) failed to pull',
);

// Both images should have been attempted
expect(execa).toHaveBeenCalledTimes(2);
expect(mockExeca).toHaveBeenCalledTimes(2);
});

it('should handle non-Error rejection', async () => {
execa.mockRejectedValueOnce('string error').mockResolvedValueOnce({ stdout: '', stderr: '' });
mockExeca.mockRejectedValueOnce('string error').mockResolvedValueOnce({ stdout: '', stderr: '' });

try {
await predownloadCommand(defaults);
Expand Down
2 changes: 1 addition & 1 deletion src/commands/predownload.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import execa from 'execa';
import { execa } from 'execa';
import { logger } from '../logger';

export interface PredownloadOptions {
Expand Down
11 changes: 5 additions & 6 deletions src/docker-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ import * as os from 'os';
const mockExecaFn = jest.fn();
const mockExecaSync = jest.fn();

// Mock execa module
jest.mock('execa', () => {
const fn = (...args: any[]) => mockExecaFn(...args);
fn.sync = (...args: any[]) => mockExecaSync(...args);
return fn;
});
// Mock execa module (v9: named exports instead of default)
jest.mock('execa', () => ({
execa: (...args: any[]) => mockExecaFn(...args),
execaSync: (...args: any[]) => mockExecaSync(...args),
}));

describe('docker-manager', () => {
describe('subnetsOverlap', () => {
Expand Down
20 changes: 10 additions & 10 deletions src/docker-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
import * as yaml from 'js-yaml';
import execa from 'execa';
import { execa, execaSync } from 'execa';
import { DockerComposeConfig, WrapperConfig, BlockedTarget, API_PROXY_PORTS, API_PROXY_HEALTH_PORT, CLI_PROXY_PORT } from './types';
import { logger } from './logger';
import { generateSquidConfig, generatePolicyManifest } from './squid-config';
Expand Down Expand Up @@ -1002,7 +1002,7 @@ export function generateDockerCompose(
if (hostsContent.includes(domain)) continue;

try {
const { stdout } = execa.sync('getent', ['hosts', domain], { timeout: 5000 });
const { stdout } = execaSync('getent', ['hosts', domain], { timeout: 5000 });
const parts = stdout.trim().split(/\s+/);
const ip = parts[0];
if (ip) {
Expand All @@ -1021,7 +1021,7 @@ export function generateDockerCompose(
// to connect to the MCP gateway running on the host.
if (config.enableHostAccess) {
try {
const { stdout } = execa.sync('docker', [
const { stdout } = execaSync('docker', [
'network', 'inspect', 'bridge',
'-f', '{{(index .IPAM.Config 0).Gateway}}'
]);
Expand Down Expand Up @@ -2538,7 +2538,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
// Just fix permissions so they're readable for artifact upload
if (fs.existsSync(sessionStateDir)) {
try {
execa.sync('chmod', ['-R', 'a+rX', sessionStateDir]);
execaSync('chmod', ['-R', 'a+rX', sessionStateDir]);
logger.info(`Agent session state available at: ${sessionStateDir}`);
} catch (error) {
logger.debug('Could not fix session state permissions:', error);
Expand All @@ -2564,7 +2564,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
const apiProxyLogsDir = path.join(proxyLogsDir, 'api-proxy-logs');
if (fs.existsSync(apiProxyLogsDir)) {
try {
execa.sync('chmod', ['-R', 'a+rX', apiProxyLogsDir]);
execaSync('chmod', ['-R', 'a+rX', apiProxyLogsDir]);
logger.info(`API proxy logs available at: ${apiProxyLogsDir}`);
} catch (error) {
logger.debug('Could not fix api-proxy log permissions:', error);
Expand All @@ -2589,7 +2589,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
const cliProxyLogsDir = path.join(proxyLogsDir, 'cli-proxy-logs');
if (fs.existsSync(cliProxyLogsDir)) {
try {
execa.sync('chmod', ['-R', 'a+rX', cliProxyLogsDir]);
execaSync('chmod', ['-R', 'a+rX', cliProxyLogsDir]);
logger.info(`CLI proxy logs available at: ${cliProxyLogsDir}`);
} catch (error) {
logger.debug('Could not fix cli-proxy log permissions:', error);
Expand All @@ -2613,7 +2613,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
// Logs were written directly to proxyLogsDir during runtime (timeout-safe)
// Just fix permissions so they're readable
try {
execa.sync('chmod', ['-R', 'a+rX', proxyLogsDir]);
execaSync('chmod', ['-R', 'a+rX', proxyLogsDir]);
logger.info(`Squid logs available at: ${proxyLogsDir}`);
} catch (error) {
logger.debug('Could not fix squid log permissions:', error);
Expand All @@ -2630,7 +2630,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
// Make logs readable by GitHub Actions runner for artifact upload
// Squid creates logs as 'proxy' user (UID 13) which runner cannot read
// chmod a+rX sets read for all users, and execute for dirs (capital X)
execa.sync('chmod', ['-R', 'a+rX', squidLogsDestination]);
execaSync('chmod', ['-R', 'a+rX', squidLogsDestination]);

logger.info(`Squid logs preserved at: ${squidLogsDestination}`);
} catch (error) {
Expand All @@ -2644,7 +2644,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
// User-specified audit dir: just fix permissions
if (fs.existsSync(auditDir)) {
try {
execa.sync('chmod', ['-R', 'a+rX', auditDir]);
execaSync('chmod', ['-R', 'a+rX', auditDir]);
logger.info(`Audit artifacts available at: ${auditDir}`);
} catch (error) {
logger.debug('Could not fix audit dir permissions:', error);
Expand All @@ -2657,7 +2657,7 @@ export async function cleanup(workDir: string, keepFiles: boolean, proxyLogsDir?
if (fs.existsSync(defaultAuditDir) && fs.readdirSync(defaultAuditDir).length > 0) {
try {
fs.renameSync(defaultAuditDir, auditDestination);
execa.sync('chmod', ['-R', 'a+rX', auditDestination]);
execaSync('chmod', ['-R', 'a+rX', auditDestination]);
logger.info(`Audit artifacts preserved at: ${auditDestination}`);
} catch (error) {
logger.debug('Could not preserve audit artifacts:', error);
Expand Down
2 changes: 1 addition & 1 deletion src/host-iptables.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ensureFirewallNetwork, setupHostIptables, cleanupHostIptables, cleanupFirewallNetwork, _resetIpv6State, HostAccessConfig, isValidPortSpec } from './host-iptables';
import execa from 'execa';
import { execa } from 'execa';

// Mock execa
jest.mock('execa');
Expand Down
2 changes: 1 addition & 1 deletion src/host-iptables.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import execa from 'execa';
import { execa } from 'execa';
import { logger } from './logger';
import { API_PROXY_PORTS } from './types';
import { DEFAULT_DNS_SERVERS } from './dns-resolver';
Expand Down
46 changes: 21 additions & 25 deletions src/jest-esm-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,20 @@
/**
* Test to verify Jest ESM configuration is working correctly.
*
* IMPORTANT: This is preparatory infrastructure for future ESM-only dependency upgrades.
*
* Current state:
* - Uses chalk v4 (has CJS support) because this PR only adds ESM transformation infrastructure
* - The Babel + transformIgnorePatterns configuration is in place and ready
* - Actual ESM-only upgrades (chalk 5.x, execa 9.x, commander 14.x) require separate PRs
* to address breaking API changes in those packages
*
* Future validation:
* - When upgrading to chalk v5+ (ESM-only), this test will validate transformation works
* - The infrastructure added here (babel.config.js, jest.config.js changes) will enable
* those upgrades without additional Jest configuration changes
*
* Validates that ESM-only packages (chalk v5, execa v9) are properly
* transformed and resolved in the Jest test environment via:
* - babel-jest transformation of ESM → CJS
* - transformIgnorePatterns allowing ESM packages to be transformed
* - Custom jest-resolver.js handling exports maps with only "import" conditions
*/

// Import chalk (ESM-only in v5+, currently v4 with CJS support)
// Import ESM-only packages to validate transformation pipeline
import chalk from 'chalk';
import { execa } from 'execa';

describe('Jest ESM Configuration', () => {
describe('chalk ESM compatibility (preparatory)', () => {
describe('chalk v5 ESM compatibility', () => {
it('should be able to import chalk', () => {
// Validates infrastructure is in place for ESM imports
// Currently uses chalk v4 (CJS), will work with v5 (ESM-only) when upgraded
expect(chalk).toBeDefined();
expect(typeof chalk.blue).toBe('function');
expect(typeof chalk.red).toBe('function');
Expand All @@ -36,27 +28,31 @@ describe('Jest ESM Configuration', () => {
});
});

describe('execa v9 ESM compatibility', () => {
it('should be able to import execa named export', () => {
expect(execa).toBeDefined();
expect(typeof execa).toBe('function');
});
});

describe('transformIgnorePatterns configuration', () => {
it('should be configured to transform ESM packages in node_modules', () => {
// Verifies jest.config.js has transformIgnorePatterns set up correctly
// This ensures babel-jest will transform chalk/execa/commander when they're ESM-only
// so babel-jest transforms chalk/execa/commander and their transitive deps
expect(chalk).toBeDefined();

// Infrastructure is ready - actual ESM validation will happen when dependencies upgrade

const result = chalk.green('success');
expect(result).toBeTruthy();
});
});

describe('babel configuration', () => {
it('should be configured for ESM→CJS transformation', () => {
// Verifies babel.config.js exists and is properly configured
// with @babel/preset-env targeting current Node.js

// This infrastructure will enable future ESM-only dependency upgrades
// Verifies babel.config.js and jest-resolver.js work together
// to transform and resolve ESM-only packages
expect(chalk).toBeDefined();
expect(chalk.blue).toBeDefined();

const text = 'ESM transformation infrastructure ready';
const coloredText = chalk.blue(text);
expect(coloredText).toContain(text);
Expand Down
Loading
Loading