Add native Codex hooks integration (#2319)
* Add native Codex hooks integration * Address Codex review feedback * Use durable Codex marketplace root * Address Codex file context review feedback * Harden Codex installer review paths * Report Codex legacy cleanup failures * fix: keep MCP manifests in marketplace sync * fix: bundle zod in MCP server * fix: warn on Codex legacy cleanup failure * Fix hook observation readiness timeouts * Address Codex hook review notes * Tighten Codex MCP file context matching * Resolve final Codex review nits * Add Codex marketplace version guidance * Reset worker failure counter on API fallback * Fix Codex cat flag file extraction
This commit is contained in:
Alex Newman
@@ -0,0 +1,63 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
import { extractFilePaths } from '../../../src/cli/adapters/codex-file-context.js';
|
||||
|
||||
let tmpDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = mkdtempSync(join(tmpdir(), 'codex-file-context-'));
|
||||
writeFileSync(join(tmpDir, 'README.md'), 'readme');
|
||||
writeFileSync(join(tmpDir, 'src.ts'), 'source');
|
||||
writeFileSync(join(tmpDir, 'notes.txt'), 'notes');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('extractFilePaths', () => {
|
||||
it('extracts existing files from Codex Bash read commands', () => {
|
||||
const paths = extractFilePaths('Bash', {
|
||||
command: 'cat README.md && head -n 20 src.ts && cat missing.md',
|
||||
}, tmpDir);
|
||||
|
||||
expect(paths).toEqual(['README.md', 'src.ts']);
|
||||
});
|
||||
|
||||
it('does not consume cat boolean flags as file arguments', () => {
|
||||
const paths = extractFilePaths('Bash', {
|
||||
command: 'cat -n README.md',
|
||||
}, tmpDir);
|
||||
|
||||
expect(paths).toEqual(['README.md']);
|
||||
});
|
||||
|
||||
it('ignores non-read Bash commands', () => {
|
||||
const paths = extractFilePaths('Bash', {
|
||||
command: 'rm README.md; echo src.ts',
|
||||
}, tmpDir);
|
||||
|
||||
expect(paths).toEqual([]);
|
||||
});
|
||||
|
||||
it('extracts MCP read tool path arrays', () => {
|
||||
const paths = extractFilePaths('mcp__local_filesystem__read_file', {
|
||||
paths: ['README.md', 'notes.txt', 'missing.txt'],
|
||||
}, tmpDir);
|
||||
|
||||
expect(paths).toEqual(['README.md', 'notes.txt']);
|
||||
});
|
||||
|
||||
it('extracts MCP exact read/view/cat tool names', () => {
|
||||
expect(extractFilePaths('mcp__fs__read', { path: 'README.md' }, tmpDir)).toEqual(['README.md']);
|
||||
expect(extractFilePaths('mcp__fs__view_files', { paths: ['README.md'] }, tmpDir)).toEqual(['README.md']);
|
||||
});
|
||||
|
||||
it('ignores MCP tool names that only contain read verbs as a prefix', () => {
|
||||
expect(extractFilePaths('mcp__fs__read_write', { path: 'README.md' }, tmpDir)).toEqual([]);
|
||||
expect(extractFilePaths('mcp__server__readonly', { path: 'README.md' }, tmpDir)).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -18,8 +18,12 @@ mock.module('../../../src/shared/hook-settings.js', () => ({
|
||||
}));
|
||||
|
||||
let mockExtractedMessage: string = '';
|
||||
let extractCallCount = 0;
|
||||
mock.module('../../../src/shared/transcript-parser.js', () => ({
|
||||
extractLastMessage: () => mockExtractedMessage,
|
||||
extractLastMessage: () => {
|
||||
extractCallCount += 1;
|
||||
return mockExtractedMessage;
|
||||
},
|
||||
}));
|
||||
|
||||
const workerCallLog: Array<{ path: string; method: string; body: any }> = [];
|
||||
@@ -44,6 +48,7 @@ let loggerSpies: ReturnType<typeof spyOn>[] = [];
|
||||
beforeEach(() => {
|
||||
workerCallLog.length = 0;
|
||||
mockExtractedMessage = '';
|
||||
extractCallCount = 0;
|
||||
loggerSpies = [
|
||||
spyOn(logger, 'info').mockImplementation(() => {}),
|
||||
spyOn(logger, 'debug').mockImplementation(() => {}),
|
||||
@@ -72,6 +77,39 @@ function postedBody(): any {
|
||||
}
|
||||
|
||||
describe('summarizeHandler — privacy tag stripping', () => {
|
||||
it('uses Codex lastAssistantMessage directly without reading a transcript', async () => {
|
||||
const { summarizeHandler } = await import('../../../src/cli/handlers/summarize.js');
|
||||
const result = await summarizeHandler.execute({
|
||||
sessionId: 'sess-codex',
|
||||
cwd: '/tmp',
|
||||
platform: 'codex',
|
||||
lastAssistantMessage: 'Codex answer <private>SECRET</private>',
|
||||
});
|
||||
|
||||
expect(result.continue).toBe(true);
|
||||
expect(extractCallCount).toBe(0);
|
||||
const body = postedBody();
|
||||
expect(body.last_assistant_message).toBe('Codex answer');
|
||||
expect(body.platformSource).toBe('codex');
|
||||
});
|
||||
|
||||
it('short-circuits Codex stop hook re-entry', async () => {
|
||||
const { summarizeHandler } = await import('../../../src/cli/handlers/summarize.js');
|
||||
const result = await summarizeHandler.execute({
|
||||
sessionId: 'sess-codex',
|
||||
cwd: '/tmp',
|
||||
platform: 'codex',
|
||||
stopHookActive: true,
|
||||
lastAssistantMessage: 'ignored',
|
||||
});
|
||||
|
||||
expect(result.continue).toBe(true);
|
||||
expect(result.suppressOutput).toBe(true);
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(extractCallCount).toBe(0);
|
||||
expect(workerCallLog).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('strips <private> tags and their content from last_assistant_message', async () => {
|
||||
mockExtractedMessage = 'Hello <private>SECRET-VALUE-42</private> world';
|
||||
|
||||
|
||||
@@ -1,12 +1,28 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test';
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
describe('Hook Lifecycle - Event Handlers', () => {
|
||||
describe('worker fallback failure counter', () => {
|
||||
it('resets stale unreachable state before 429/5xx API fallbacks', () => {
|
||||
const source = readFileSync('src/shared/worker-utils.ts', 'utf-8');
|
||||
const nonOkRegion = source.slice(
|
||||
source.indexOf('if (!response.ok)'),
|
||||
source.indexOf('const text = await response.text();'),
|
||||
);
|
||||
|
||||
expect(nonOkRegion.indexOf('resetWorkerFailureCounter()'))
|
||||
.toBeLessThan(nonOkRegion.indexOf('response.status === 429 || response.status >= 500'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('getEventHandler', () => {
|
||||
it('should return handler for all recognized event types', async () => {
|
||||
const { getEventHandler } = await import('../src/cli/handlers/index.js');
|
||||
const recognizedTypes = [
|
||||
'context', 'session-init', 'observation',
|
||||
'summarize', 'user-message', 'file-edit'
|
||||
'summarize', 'user-message', 'file-edit', 'file-context'
|
||||
];
|
||||
for (const type of recognizedTypes) {
|
||||
const handler = getEventHandler(type);
|
||||
@@ -35,10 +51,10 @@ describe('Hook Lifecycle - Event Handlers', () => {
|
||||
|
||||
describe('Codex CLI Compatibility (#744)', () => {
|
||||
describe('getPlatformAdapter', () => {
|
||||
it('should return rawAdapter for unknown platforms like codex', async () => {
|
||||
const { getPlatformAdapter, rawAdapter } = await import('../src/cli/adapters/index.js');
|
||||
it('should return codexAdapter for codex', async () => {
|
||||
const { getPlatformAdapter, codexAdapter } = await import('../src/cli/adapters/index.js');
|
||||
const adapter = getPlatformAdapter('codex');
|
||||
expect(adapter).toBe(rawAdapter);
|
||||
expect(adapter).toBe(codexAdapter);
|
||||
});
|
||||
|
||||
it('should return rawAdapter for any unrecognized platform string', async () => {
|
||||
@@ -81,6 +97,120 @@ describe('Codex CLI Compatibility (#744)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('codexAdapter', () => {
|
||||
it('normalizes snake_case Stop payloads with last assistant message', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const input = codexAdapter.normalizeInput({
|
||||
hook_event_name: 'Stop',
|
||||
session_id: 'codex-session',
|
||||
turn_id: 'turn-1',
|
||||
cwd: '/tmp',
|
||||
stop_hook_active: false,
|
||||
last_assistant_message: 'done',
|
||||
});
|
||||
|
||||
expect(input.sessionId).toBe('codex-session');
|
||||
expect(input.turnId).toBe('turn-1');
|
||||
expect(input.lastAssistantMessage).toBe('done');
|
||||
expect(input.stopHookActive).toBe(false);
|
||||
});
|
||||
|
||||
it('normalizes string stop_hook_active payloads', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const active = codexAdapter.normalizeInput({
|
||||
hook_event_name: 'Stop',
|
||||
session_id: 'codex-session',
|
||||
cwd: '/tmp',
|
||||
stop_hook_active: 'true',
|
||||
});
|
||||
const inactive = codexAdapter.normalizeInput({
|
||||
hook_event_name: 'Stop',
|
||||
session_id: 'codex-session',
|
||||
cwd: '/tmp',
|
||||
stop_hook_active: 'false',
|
||||
});
|
||||
|
||||
expect(active.stopHookActive).toBe(true);
|
||||
expect(inactive.stopHookActive).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects payloads without a session_id', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const { AdapterRejectedInput } = await import('../src/cli/adapters/errors.js');
|
||||
|
||||
expect(() => codexAdapter.normalizeInput({
|
||||
hook_event_name: 'Stop',
|
||||
cwd: '/tmp',
|
||||
})).toThrow(AdapterRejectedInput);
|
||||
});
|
||||
|
||||
it('adds filePaths without dropping the original object tool input', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const tmpDir = mkdtempSync(join(tmpdir(), 'codex-adapter-'));
|
||||
try {
|
||||
writeFileSync(join(tmpDir, 'README.md'), 'readme');
|
||||
|
||||
const input = codexAdapter.normalizeInput({
|
||||
hook_event_name: 'PreToolUse',
|
||||
session_id: 'codex-session',
|
||||
cwd: tmpDir,
|
||||
tool_name: 'Bash',
|
||||
tool_input: { command: 'cat README.md' },
|
||||
});
|
||||
|
||||
expect(input.toolInput).toEqual({
|
||||
command: 'cat README.md',
|
||||
filePaths: ['README.md'],
|
||||
});
|
||||
} finally {
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves non-object tool input payloads', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const input = codexAdapter.normalizeInput({
|
||||
hook_event_name: 'PreToolUse',
|
||||
session_id: 'codex-session',
|
||||
cwd: '/tmp',
|
||||
tool_name: 'Bash',
|
||||
tool_input: 'cat README.md',
|
||||
});
|
||||
|
||||
expect(input.toolInput).toBe('cat README.md');
|
||||
});
|
||||
|
||||
it('drops PreToolUse allow decisions because Codex only accepts deny', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const output = codexAdapter.formatOutput({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
additionalContext: 'file history',
|
||||
permissionDecision: 'allow',
|
||||
},
|
||||
}) as any;
|
||||
|
||||
expect(output.hookSpecificOutput).toEqual({
|
||||
hookEventName: 'PreToolUse',
|
||||
additionalContext: 'file history',
|
||||
});
|
||||
});
|
||||
|
||||
it('does not emit hookSpecificOutput for Stop outputs', async () => {
|
||||
const { codexAdapter } = await import('../src/cli/adapters/codex.js');
|
||||
const output = codexAdapter.formatOutput({
|
||||
continue: true,
|
||||
suppressOutput: true,
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'Stop',
|
||||
additionalContext: 'ignored',
|
||||
},
|
||||
}) as any;
|
||||
|
||||
expect(output).toEqual({ continue: true, suppressOutput: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('session-init handler undefined prompt', () => {
|
||||
it('should not throw when prompt is undefined', () => {
|
||||
const rawPrompt: string | undefined = undefined;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, spyOn, mock } from 'bun:test';
|
||||
import { mkdtempSync, writeFileSync, utimesSync, rmSync } from 'fs';
|
||||
import { mkdirSync, mkdtempSync, writeFileSync, utimesSync, rmSync } from 'fs';
|
||||
import { tmpdir, homedir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
@@ -181,4 +181,74 @@ describe('fileContextHandler — #2094 (no Read mutation)', () => {
|
||||
expect(ctx).not.toContain('Only line 1 was read');
|
||||
expect(ctx).toContain('full requested section');
|
||||
});
|
||||
|
||||
it('accepts a Codex filePaths array and joins per-file context blocks', async () => {
|
||||
const otherFile = join(tmpDir, 'other.md');
|
||||
writeFileSync(otherFile, PADDING);
|
||||
|
||||
const future = Date.now() + 60_000;
|
||||
fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((url: string | URL | Request) => {
|
||||
const text = String(url);
|
||||
if (text.includes('other.md')) {
|
||||
return Promise.resolve(makeObservationsResponse([{ id: 2, created_at_epoch: future, title: 'Other file context' }]));
|
||||
}
|
||||
return Promise.resolve(makeObservationsResponse([{ id: 1, created_at_epoch: future, title: 'Main file context' }]));
|
||||
});
|
||||
|
||||
const result = await fileContextHandler.execute({
|
||||
sessionId: 'sess',
|
||||
cwd: tmpDir,
|
||||
toolName: 'Bash',
|
||||
toolInput: { filePaths: [testFile, otherFile] },
|
||||
});
|
||||
|
||||
const ctx = result.hookSpecificOutput!.additionalContext as string;
|
||||
expect(ctx).toContain('Main file context');
|
||||
expect(ctx).toContain('Other file context');
|
||||
expect(ctx).toContain('\n\n---\n\n');
|
||||
});
|
||||
|
||||
it('keeps successful timelines when one file lookup fails', async () => {
|
||||
const otherFile = join(tmpDir, 'other.md');
|
||||
writeFileSync(otherFile, PADDING);
|
||||
|
||||
const future = Date.now() + 60_000;
|
||||
fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((url: string | URL | Request) => {
|
||||
const text = String(url);
|
||||
if (text.includes('other.md')) {
|
||||
return Promise.reject(new Error('worker unavailable'));
|
||||
}
|
||||
return Promise.resolve(makeObservationsResponse([{ id: 1, created_at_epoch: future, title: 'Main file context' }]));
|
||||
});
|
||||
|
||||
const result = await fileContextHandler.execute({
|
||||
sessionId: 'sess',
|
||||
cwd: tmpDir,
|
||||
toolName: 'Bash',
|
||||
toolInput: { filePaths: [testFile, otherFile] },
|
||||
});
|
||||
|
||||
const ctx = result.hookSpecificOutput!.additionalContext as string;
|
||||
expect(ctx).toContain('Main file context');
|
||||
expect(ctx).not.toContain('worker unavailable');
|
||||
});
|
||||
|
||||
it('skips directories before querying file history', async () => {
|
||||
const directoryPath = join(tmpDir, 'large-dir');
|
||||
mkdirSync(directoryPath);
|
||||
fetchSpy = spyOn(globalThis, 'fetch').mockResolvedValue(
|
||||
makeObservationsResponse([{ id: 1, created_at_epoch: Date.now() + 60_000 }])
|
||||
);
|
||||
|
||||
const result = await fileContextHandler.execute({
|
||||
sessionId: 'sess',
|
||||
cwd: tmpDir,
|
||||
toolName: 'Bash',
|
||||
toolInput: { filePaths: [directoryPath] },
|
||||
});
|
||||
|
||||
expect(result.continue).toBe(true);
|
||||
expect(result.hookSpecificOutput).toBeUndefined();
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,6 +11,22 @@ const installSourcePath = join(
|
||||
'install.ts',
|
||||
);
|
||||
const installSource = readFileSync(installSourcePath, 'utf-8');
|
||||
const codexInstallerSourcePath = join(
|
||||
__dirname,
|
||||
'..',
|
||||
'src',
|
||||
'services',
|
||||
'integrations',
|
||||
'CodexCliInstaller.ts',
|
||||
);
|
||||
const codexInstallerSource = readFileSync(codexInstallerSourcePath, 'utf-8');
|
||||
const syncMarketplaceSourcePath = join(
|
||||
__dirname,
|
||||
'..',
|
||||
'scripts',
|
||||
'sync-marketplace.cjs',
|
||||
);
|
||||
const syncMarketplaceSource = readFileSync(syncMarketplaceSourcePath, 'utf-8');
|
||||
|
||||
describe('Install Non-TTY Support', () => {
|
||||
describe('isInteractive flag', () => {
|
||||
@@ -76,6 +92,76 @@ describe('Install Non-TTY Support', () => {
|
||||
it('uses console.log for note/summary in non-interactive mode', () => {
|
||||
expect(installSource).toContain("console.log(`\\n ${installStatus}`)");
|
||||
});
|
||||
|
||||
it('copies Codex marketplace metadata to the durable marketplace directory', () => {
|
||||
const copyRegion = installSource.slice(
|
||||
installSource.indexOf('const allowedTopLevelEntries = ['),
|
||||
installSource.indexOf('function copyPluginToCache'),
|
||||
);
|
||||
expect(copyRegion).toContain("'.agents'");
|
||||
expect(copyRegion).toContain("'.codex-plugin'");
|
||||
expect(copyRegion).toContain("'.mcp.json'");
|
||||
});
|
||||
|
||||
it('does not exclude MCP manifests during local marketplace sync', () => {
|
||||
const gitignoreExcludeRegion = syncMarketplaceSource.slice(
|
||||
syncMarketplaceSource.indexOf('function getGitignoreExcludes'),
|
||||
syncMarketplaceSource.indexOf('const branch = getCurrentBranch'),
|
||||
);
|
||||
expect(gitignoreExcludeRegion).toContain("'.mcp.json'");
|
||||
expect(gitignoreExcludeRegion).toContain('syncManagedFiles.has(line)');
|
||||
});
|
||||
|
||||
it('registers Codex against the durable marketplace directory', () => {
|
||||
expect(installSource).toContain('installCodexCli(marketplaceDirectory())');
|
||||
});
|
||||
|
||||
it('captures Codex CLI output for install failure reporting', () => {
|
||||
const runCodexRegion = codexInstallerSource.slice(
|
||||
codexInstallerSource.indexOf('function runCodex'),
|
||||
codexInstallerSource.indexOf('function removeCodexAgentsMdContext'),
|
||||
);
|
||||
expect(runCodexRegion).toContain('spawnSync');
|
||||
expect(runCodexRegion).not.toContain("stdio: 'inherit'");
|
||||
});
|
||||
|
||||
it('checks Codex CLI marketplace version before registration', () => {
|
||||
const installRegion = codexInstallerSource.slice(
|
||||
codexInstallerSource.indexOf('export async function installCodexCli'),
|
||||
codexInstallerSource.indexOf('export function uninstallCodexCli'),
|
||||
);
|
||||
expect(codexInstallerSource).toContain("const MIN_CODEX_MARKETPLACE_VERSION = '0.128.0'");
|
||||
expect(codexInstallerSource).toContain("spawnSync('codex', ['--version']");
|
||||
expect(installRegion.indexOf('assertCodexMarketplaceSupported()'))
|
||||
.toBeLessThan(installRegion.indexOf("runCodex(['plugin', 'marketplace', 'add', marketplaceRoot])"));
|
||||
});
|
||||
|
||||
it('removes legacy Codex AGENTS context only after marketplace registration succeeds', () => {
|
||||
const installRegion = codexInstallerSource.slice(
|
||||
codexInstallerSource.indexOf('export async function installCodexCli'),
|
||||
codexInstallerSource.indexOf('export function uninstallCodexCli'),
|
||||
);
|
||||
expect(installRegion.indexOf("runCodex(['plugin', 'marketplace', 'add', marketplaceRoot])"))
|
||||
.toBeLessThan(installRegion.indexOf('cleanupLegacyCodexAgentsMdContext()'));
|
||||
});
|
||||
|
||||
it('reports legacy Codex AGENTS cleanup failures to callers', () => {
|
||||
expect(codexInstallerSource).toContain('function removeCodexAgentsMdContext(): boolean');
|
||||
expect(codexInstallerSource).toContain('if (!cleanupLegacyCodexAgentsMdContext())');
|
||||
});
|
||||
|
||||
it('does not fail Codex install after marketplace registration when only AGENTS cleanup fails', () => {
|
||||
const installRegion = codexInstallerSource.slice(
|
||||
codexInstallerSource.indexOf('export async function installCodexCli'),
|
||||
codexInstallerSource.indexOf('export function uninstallCodexCli'),
|
||||
);
|
||||
const cleanupFailureRegion = installRegion.slice(
|
||||
installRegion.indexOf('if (!cleanupLegacyCodexAgentsMdContext())'),
|
||||
installRegion.indexOf('Installation complete!'),
|
||||
);
|
||||
expect(cleanupFailureRegion).toContain('console.warn');
|
||||
expect(cleanupFailureRegion).not.toContain('return 1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('TaskDescriptor interface', () => {
|
||||
|
||||
Reference in New Issue
Block a user