diff --git a/README.md b/README.md index ee952317..93f169e9 100644 --- a/README.md +++ b/README.md @@ -222,7 +222,7 @@ To route summarization through a custom Anthropic-compatible endpoint or overrid # Override model (default: haiku) export EPISODIC_MEMORY_API_MODEL=opus -# Override fallback model on error (default: sonnet) +# Override the Claude fallback model on error (default: sonnet; not used for Codex) export EPISODIC_MEMORY_API_MODEL_FALLBACK=sonnet # Route through custom endpoint @@ -294,7 +294,10 @@ These pass through unchanged to episodic-memory's summarizer subprocess. Because These settings only affect episodic-memory's summarization calls, not your interactive Claude Code or Codex sessions. -Codex summarization requires `codex-cli 0.130.0` or newer. If Codex app-server summarization is unavailable, sync logs the reason and falls back to transcript-text summarization. +Codex summarization requires `codex-cli 0.130.0` or newer and a session ID for +an ephemeral read-only fork. If that route is unavailable, the summary attempt +fails and can be retried later; it does not silently switch to Claude or a +metered Anthropic API. Indexing exchanges remains independent of the summary. ### What's Affected diff --git a/dist/summarizer.d.ts b/dist/summarizer.d.ts index 4c97be52..30041f0b 100644 --- a/dist/summarizer.d.ts +++ b/dist/summarizer.d.ts @@ -166,8 +166,8 @@ export declare function runCodexCommand(command: CodexSummarizerCommand): Promis * * Default to `undefined` so `app-server` uses the current Codex config * (`~/.codex/config.toml#model`). Operators can override via - * `EPISODIC_MEMORY_CODEX_MODEL` if they need a specific model id (e.g. an - * API-key user wanting `gpt-5.5-codex`). + * `EPISODIC_MEMORY_CODEX_MODEL` if they need a supported model id for their + * selected Codex route. * * See https://github.com/obra/episodic-memory/issues/98. */ diff --git a/dist/summarizer.js b/dist/summarizer.js index 60719809..bd814b60 100644 --- a/dist/summarizer.js +++ b/dist/summarizer.js @@ -609,8 +609,8 @@ function getCodexSessionId(exchanges, sessionId) { * * Default to `undefined` so `app-server` uses the current Codex config * (`~/.codex/config.toml#model`). Operators can override via - * `EPISODIC_MEMORY_CODEX_MODEL` if they need a specific model id (e.g. an - * API-key user wanting `gpt-5.5-codex`). + * `EPISODIC_MEMORY_CODEX_MODEL` if they need a supported model id for their + * selected Codex route. * * See https://github.com/obra/episodic-memory/issues/98. */ @@ -628,20 +628,19 @@ export async function summarizeConversation(exchanges, sessionId) { return 'Trivial conversation with no substantive content.'; } } - const codexSessionId = getCodexSessionId(exchanges, sessionId); - if (codexSessionId) { - try { - const result = await callCodex(buildCodexSummaryPrompt(), codexSessionId, getCodexModel(exchanges)); - return extractSummary(result); - } - catch (error) { - console.log(` Codex summarizer unavailable, falling back to transcript text: ${error instanceof Error ? error.message : String(error)}`); + const isCodexConversation = exchanges.some(exchange => exchange.harness === 'codex'); + if (isCodexConversation) { + const codexSessionId = getCodexSessionId(exchanges, sessionId); + if (!codexSessionId) { + throw new Error('Codex session ID is required for a read-only summary fork'); } + const result = await callCodex(buildCodexSummaryPrompt(), codexSessionId, getCodexModel(exchanges)); + return extractSummary(result); } // Cost safety (#104): everything below this point calls the metered Claude API - // path. If a stray global ANTHROPIC_API_KEY would be billed and the user hasn't + // path. Codex conversations return or fail before reaching this point. + // If a stray global ANTHROPIC_API_KEY would be billed and the user hasn't // acknowledged it, warn once and proceed (don't silently spend without notice). - // Placed AFTER the Codex attempt so pure-Codex users are never warned needlessly. if (wouldBillMeteredApi() && !meteredApiOptIn()) { warnMeteredApiOnce(); } @@ -652,7 +651,7 @@ export async function summarizeConversation(exchanges, sessionId) { // would fail on every one before the no-resume retry kicks in. Treat // missing harness as Claude for backward compatibility with old archives. const isClaudeSession = exchanges.some(e => e.harness === 'claude' || e.harness === undefined); - const claudeSessionId = !codexSessionId && isClaudeSession ? sessionId : undefined; + const claudeSessionId = isClaudeSession ? sessionId : undefined; const cwd = claudeSessionId ? exchanges.find(e => e.cwd)?.cwd : undefined; const conversationText = claudeSessionId ? '' // When resuming, no need to include conversation text - it's already in context diff --git a/docs/CODEX.md b/docs/CODEX.md index c3d83f97..1bafc40e 100644 --- a/docs/CODEX.md +++ b/docs/CODEX.md @@ -92,4 +92,7 @@ The test creates an isolated temporary `CODEX_HOME`, copies your existing Codex Codex summaries use `codex app-server`, `thread/fork`, and `ephemeral: true`. This matters: `codex exec --ephemeral resume ` was tested and still appended to the resumed rollout, so it is not the quality bar for summarization. -If the Codex app-server summarizer is unavailable or below the support floor, Episodic Memory logs the reason and falls back to transcript-text summarization instead of silently skipping the conversation. +If the Codex app-server summarizer is unavailable, below the support floor, or +the transcript has no session ID to fork, the summary attempt fails with a +retryable error. It does not switch providers to Claude. Conversation exchanges +can still be indexed and searched without a generated summary. diff --git a/src/summarizer.ts b/src/summarizer.ts index d7e3cc04..988124e1 100644 --- a/src/summarizer.ts +++ b/src/summarizer.ts @@ -693,8 +693,8 @@ function getCodexSessionId(exchanges: ConversationExchange[], sessionId?: string * * Default to `undefined` so `app-server` uses the current Codex config * (`~/.codex/config.toml#model`). Operators can override via - * `EPISODIC_MEMORY_CODEX_MODEL` if they need a specific model id (e.g. an - * API-key user wanting `gpt-5.5-codex`). + * `EPISODIC_MEMORY_CODEX_MODEL` if they need a supported model id for their + * selected Codex route. * * See https://github.com/obra/episodic-memory/issues/98. */ @@ -715,20 +715,20 @@ export async function summarizeConversation(exchanges: ConversationExchange[], s } } - const codexSessionId = getCodexSessionId(exchanges, sessionId); - if (codexSessionId) { - try { - const result = await callCodex(buildCodexSummaryPrompt(), codexSessionId, getCodexModel(exchanges)); - return extractSummary(result); - } catch (error) { - console.log(` Codex summarizer unavailable, falling back to transcript text: ${error instanceof Error ? error.message : String(error)}`); + const isCodexConversation = exchanges.some(exchange => exchange.harness === 'codex'); + if (isCodexConversation) { + const codexSessionId = getCodexSessionId(exchanges, sessionId); + if (!codexSessionId) { + throw new Error('Codex session ID is required for a read-only summary fork'); } + const result = await callCodex(buildCodexSummaryPrompt(), codexSessionId, getCodexModel(exchanges)); + return extractSummary(result); } // Cost safety (#104): everything below this point calls the metered Claude API - // path. If a stray global ANTHROPIC_API_KEY would be billed and the user hasn't + // path. Codex conversations return or fail before reaching this point. + // If a stray global ANTHROPIC_API_KEY would be billed and the user hasn't // acknowledged it, warn once and proceed (don't silently spend without notice). - // Placed AFTER the Codex attempt so pure-Codex users are never warned needlessly. if (wouldBillMeteredApi() && !meteredApiOptIn()) { warnMeteredApiOnce(); } @@ -742,7 +742,7 @@ export async function summarizeConversation(exchanges: ConversationExchange[], s const isClaudeSession = exchanges.some( e => e.harness === 'claude' || e.harness === undefined ); - const claudeSessionId = !codexSessionId && isClaudeSession ? sessionId : undefined; + const claudeSessionId = isClaudeSession ? sessionId : undefined; const cwd = claudeSessionId ? exchanges.find(e => e.cwd)?.cwd : undefined; const conversationText = claudeSessionId ? '' // When resuming, no need to include conversation text - it's already in context diff --git a/test/codex-no-fallback.test.ts b/test/codex-no-fallback.test.ts new file mode 100644 index 00000000..0a6332c3 --- /dev/null +++ b/test/codex-no-fallback.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import type { ConversationExchange } from '../src/types.js'; + +vi.mock('@anthropic-ai/claude-agent-sdk', () => ({ query: vi.fn() })); +import { query } from '@anthropic-ai/claude-agent-sdk'; +import { summarizeConversation } from '../src/summarizer.js'; + +describe('Codex summary provider boundary', () => { + let directory: string; + let previous: NodeJS.ProcessEnv; + + beforeEach(() => { + previous = { ...process.env }; + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'em-codex-no-fallback-')); + process.env.EPISODIC_MEMORY_CONFIG_DIR = directory; + process.env.EPISODIC_MEMORY_CODEX_BIN = path.join(directory, 'missing-codex'); + vi.mocked(query).mockReset(); + }); + + afterEach(() => { + process.env = previous; + fs.rmSync(directory, { recursive: true, force: true }); + }); + + it('does not call Claude when Codex app-server is unavailable', async () => { + vi.mocked(query).mockReturnValue({ + async *[Symbol.asyncIterator]() { + yield { type: 'result', is_error: false, result: 'Unexpected Claude fallback.' }; + }, + } as any); + const exchange: ConversationExchange = { + id: 'codex-1', project: 'fixture', timestamp: '2026-01-01T00:00:00Z', + userMessage: 'Explain the design decisions behind the memory index and the safe summary route in this project.', + assistantMessage: 'The index uses line high-water marks and summaries are generated only after provider admission.', + archivePath: path.join(directory, 'session.jsonl'), lineStart: 1, lineEnd: 2, + harness: 'codex', sessionId: '00000000-1111-4222-8333-444444444444', + }; + + await expect(summarizeConversation([exchange], exchange.sessionId)).rejects.toThrow(); + expect(query).not.toHaveBeenCalled(); + }); + + it('keeps a Codex transcript without a session ID on the Codex route', async () => { + vi.mocked(query).mockReturnValue({ + async *[Symbol.asyncIterator]() { + yield { type: 'result', is_error: false, result: 'Unexpected Claude fallback.' }; + }, + } as any); + const exchange: ConversationExchange = { + id: 'codex-no-id', project: 'fixture', timestamp: '2026-01-01T00:00:00Z', + userMessage: 'Explain the design decisions behind the memory index and the safe summary route in this project.', + assistantMessage: 'The index uses line high-water marks and summaries are generated only after provider admission.', + archivePath: path.join(directory, 'session.jsonl'), lineStart: 1, lineEnd: 2, + harness: 'codex', + }; + + await expect(summarizeConversation([exchange])).rejects.toThrow(/session ID/); + expect(query).not.toHaveBeenCalled(); + }); +});