diff --git a/lib/request/request-transformer.ts b/lib/request/request-transformer.ts index d1e4ce6..0ab4454 100644 --- a/lib/request/request-transformer.ts +++ b/lib/request/request-transformer.ts @@ -864,12 +864,27 @@ const PROMPT_CACHE_METADATA_KEYS = [ "chatId", ]; +const PROMPT_CACHE_FORK_KEYS = [ + "forkId", + "fork_id", + "branchId", + "branch_id", + "parentConversationId", + "parent_conversation_id", +]; + type PromptCacheKeySource = "existing" | "metadata" | "generated"; interface PromptCacheKeyResult { key: string; source: PromptCacheKeySource; sourceKey?: string; + forkSourceKey?: string; + hintKeys?: string[]; + unusableKeys?: string[]; + forkHintKeys?: string[]; + forkUnusableKeys?: string[]; + fallbackHash?: string; } function extractString(value: unknown): string | undefined { @@ -880,38 +895,96 @@ function extractString(value: unknown): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } -function derivePromptCacheKeyFromBody(body: RequestBody): { value: string; sourceKey: string } | undefined { +function normalizeCacheKeyBase(base: string): string { + const trimmed = base.trim(); + if (!trimmed) { + return `cache_${randomUUID()}`; + } + const sanitized = trimmed.replace(/\s+/g, "-"); + return sanitized.startsWith("cache_") ? sanitized : `cache_${sanitized}`; +} + +function normalizeForkSuffix(forkId: string): string { + const trimmed = forkId.trim(); + if (!trimmed) return "fork"; + return trimmed.replace(/\s+/g, "-"); +} + +function derivePromptCacheKeyFromBody(body: RequestBody): { + base?: string; + sourceKey?: string; + hintKeys: string[]; + unusableKeys: string[]; + forkId?: string; + forkSourceKey?: string; + forkHintKeys: string[]; + forkUnusableKeys: string[]; +} { const metadata = body.metadata as Record | undefined; const root = body as Record; - const getForkIdentifier = (): string | undefined => { - // Prefer metadata over root, and support both camelCase and snake_case - return ( - extractString(metadata?.forkId) || - extractString(metadata?.fork_id) || - extractString(metadata?.branchId) || - extractString(metadata?.branch_id) || - extractString(root.forkId) || - extractString(root.fork_id) || - extractString(root.branchId) || - extractString(root.branch_id) - ); - }; - - const forkId = getForkIdentifier(); + const hintKeys: string[] = []; + const unusableKeys: string[] = []; + let base: string | undefined; + let sourceKey: string | undefined; for (const key of PROMPT_CACHE_METADATA_KEYS) { - const base = extractString(metadata?.[key]) ?? extractString(root[key]); - if (base) { - const value = forkId ? `${base}::fork::${forkId}` : base; - return { value, sourceKey: key }; + const raw = metadata?.[key] ?? root[key]; + if (raw !== undefined) { + hintKeys.push(key); + } + const value = extractString(raw); + if (value) { + base = value; + sourceKey = key; + break; + } + if (raw !== undefined) { + unusableKeys.push(key); } } - return undefined; + + const forkHintKeys: string[] = []; + const forkUnusableKeys: string[] = []; + let forkId: string | undefined; + let forkSourceKey: string | undefined; + + for (const key of PROMPT_CACHE_FORK_KEYS) { + const raw = metadata?.[key] ?? root[key]; + if (raw !== undefined) { + forkHintKeys.push(key); + } + const value = extractString(raw); + if (value) { + forkId = value; + forkSourceKey = key; + break; + } + if (raw !== undefined) { + forkUnusableKeys.push(key); + } + } + + return { + base, + sourceKey, + hintKeys, + unusableKeys, + forkId, + forkSourceKey, + forkHintKeys, + forkUnusableKeys, + }; } -function generatePromptCacheKey(): string { - return `cache_${randomUUID()}`; +function computeFallbackHashForBody(body: RequestBody): string { + const inputSlice = Array.isArray(body.input) ? body.input.slice(0, 3) : undefined; + const seed = stableStringify({ + model: typeof body.model === "string" ? body.model : undefined, + metadata: body.metadata, + input: inputSlice, + }); + return createHash("sha1").update(seed).digest("hex").slice(0, 12); } function ensurePromptCacheKey(body: RequestBody): PromptCacheKeyResult { @@ -931,17 +1004,35 @@ function ensurePromptCacheKey(body: RequestBody): PromptCacheKeyResult { } const derived = derivePromptCacheKeyFromBody(body); - if (derived) { - const sanitized = extractString(derived.value) ?? generatePromptCacheKey(); - body.prompt_cache_key = sanitized; + if (derived.base) { + const baseKey = normalizeCacheKeyBase(derived.base); + const suffix = derived.forkId ? `-fork-${normalizeForkSuffix(derived.forkId)}` : ""; + const finalKey = `${baseKey}${suffix}`; + body.prompt_cache_key = finalKey; // Don't set camelCase field for derived keys - only snake_case for Codex - return { key: sanitized, source: "metadata", sourceKey: derived.sourceKey }; + return { + key: finalKey, + source: "metadata", + sourceKey: derived.sourceKey, + forkSourceKey: derived.forkSourceKey, + hintKeys: derived.hintKeys, + forkHintKeys: derived.forkHintKeys, + }; } - const generated = generatePromptCacheKey(); + const fallbackHash = computeFallbackHashForBody(body); + const generated = `cache_${fallbackHash}`; body.prompt_cache_key = generated; // Don't set camelCase field for generated keys - only snake_case for Codex - return { key: generated, source: "generated" }; + return { + key: generated, + source: "generated", + hintKeys: derived.hintKeys, + unusableKeys: derived.unusableKeys, + forkHintKeys: derived.forkHintKeys, + forkUnusableKeys: derived.forkUnusableKeys, + fallbackHash, + }; } /** @@ -1035,11 +1126,27 @@ export async function transformRequestBody( logDebug("Prompt cache key missing; derived from metadata", { promptCacheKey: cacheKeyResult.key, sourceKey: cacheKeyResult.sourceKey, + forkSourceKey: cacheKeyResult.forkSourceKey, + forkHintKeys: cacheKeyResult.forkHintKeys, }); } else if (cacheKeyResult.source === "generated") { - logWarn("Prompt cache key missing; generated fallback cache key", { - promptCacheKey: cacheKeyResult.key, - }); + const hasHints = Boolean( + (cacheKeyResult.hintKeys && cacheKeyResult.hintKeys.length > 0) || + (cacheKeyResult.forkHintKeys && cacheKeyResult.forkHintKeys.length > 0), + ); + logWarn( + hasHints + ? "Prompt cache key hints detected but unusable; generated fallback cache key" + : "Prompt cache key missing; generated fallback cache key", + { + promptCacheKey: cacheKeyResult.key, + fallbackHash: cacheKeyResult.fallbackHash, + hintKeys: cacheKeyResult.hintKeys, + unusableKeys: cacheKeyResult.unusableKeys, + forkHintKeys: cacheKeyResult.forkHintKeys, + forkUnusableKeys: cacheKeyResult.forkUnusableKeys, + }, + ); } // Tool behavior parity with Codex CLI (normalize shapes) diff --git a/spec/issue-4-prompt-cache-key.md b/spec/issue-4-prompt-cache-key.md new file mode 100644 index 0000000..9ee1a9c --- /dev/null +++ b/spec/issue-4-prompt-cache-key.md @@ -0,0 +1,29 @@ +# Issue 4 – Fork-aware prompt_cache_key and non-structural overrides + +**Issue**: https://github.com/open-hax/codex/issues/4 (open) + +## Context & Current Behavior +- `lib/request/request-transformer.ts:856-1043` — `ensurePromptCacheKey` now normalizes metadata-derived keys to `cache_` and appends `-fork-` when `forkId/branchId/parentConversationId` is present; otherwise derives deterministic hashed fallback `cache_`. +- `lib/request/request-transformer.ts:915-1043` — Transform pipeline logs when deriving/generating keys with hint details and fallback hashes. +- `lib/session/session-manager.ts:83-206` — SessionManager derives session IDs from conversation metadata or host-provided cache key; resets cache key on prefix mismatch; preserves prompt_cache_key continuity when possible. +- `test/request-transformer.test.ts:715-850` — Tests cover preserving host keys, metadata derivation, fork suffix (`-fork-`), stability across non-structural overrides, and deterministic fallback generation. + +## Gaps vs Issue Requirements +- Fork derivation is normalized but not yet numbered; relies on provided fork identifiers/metadata. +- Fallback keys are hashed but still lack explicit numbering for forks (pending if required later). +- Logging does not surface when fallback occurs despite having conversation-like metadata; need stronger WARN. +- No tests mirroring Codex CLI semantics for: constant keys across soft overrides, distinct keys for forks with numbering/hashing, deterministic fallback reuse across transforms. + +## Plan (Phases) +1) **Design & Hooks**: Decide fork-key schema (`cache_` + `-fork-`), define what counts as fork metadata (forkId/branchId, future parentConversationId), and how to seed numbering from metadata vs. fallback detection. +2) **Implementation**: Update `ensurePromptCacheKey` (and helpers) to: + - Normalize base cache key from metadata/host; seed fork suffix with deterministic numbering when forks requested; keep stability across soft overrides. + - Detect conversation-like hints when falling back; emit warn and include short hash of input/fallback seed (`cache_-` or similar) to reduce accidental reuse. + - Ensure SessionManager interactions remain compatible (no regressions on prefix matching). +3) **Tests & Docs**: Add unit coverage in `test/request-transformer.test.ts` (fork numbering, fallback hash stability across transforms, soft-override stability, fork distinction). Update docs if behavior changes materially (configuration/getting-started sections mentioning prompt_cache_key behavior). + +## Definition of Done / Requirements +- Prompt cache key derivation mirrors Codex CLI semantics: stable across soft overrides (temperature/max tokens/reasoning fields), distinct for explicit forks, deterministic fallback reuse for identical bodies, and warns when fallback occurs despite conversation hints. +- New/updated tests in `test/request-transformer.test.ts` cover: (a) stable key with overrides, (b) fork-specific keys with deterministic suffix/numbering, (c) fallback key reuse with hash component, (d) warning path when conversation-like metadata is unusable. +- Code builds and relevant tests pass (`pnpm test` at minimum; broader suites as needed). +- No regression to SessionManager behavior or existing prompt_cache_key consumers. diff --git a/test/request-transformer.test.ts b/test/request-transformer.test.ts index fd38e03..4f1eadb 100644 --- a/test/request-transformer.test.ts +++ b/test/request-transformer.test.ts @@ -741,7 +741,7 @@ describe('runTransform', () => { metadata: { conversation_id: 'meta-conv-123' }, }; const result: any = await runTransform(body, codexInstructions); - expect(result.prompt_cache_key).toBe('meta-conv-123'); + expect(result.prompt_cache_key).toBe('cache_meta-conv-123'); }); it('derives fork-aware prompt_cache_key when fork id is present in metadata', async () => { @@ -751,7 +751,7 @@ describe('runTransform', () => { metadata: { conversation_id: 'meta-conv-123', forkId: 'branch-1' }, }; const result: any = await runTransform(body, codexInstructions); - expect(result.prompt_cache_key).toBe('meta-conv-123::fork::branch-1'); + expect(result.prompt_cache_key).toBe('cache_meta-conv-123-fork-branch-1'); }); it('derives fork-aware prompt_cache_key when fork id is present in root', async () => { @@ -762,7 +762,7 @@ describe('runTransform', () => { forkId: 'branch-2' as any, } as any; const result: any = await runTransform(body, codexInstructions); - expect(result.prompt_cache_key).toBe('meta-conv-123::fork::branch-2'); + expect(result.prompt_cache_key).toBe('cache_meta-conv-123-fork-branch-2'); }); it('reuses the same prompt_cache_key across non-structural overrides', async () => { @@ -785,18 +785,22 @@ describe('runTransform', () => { const result1: any = await runTransform(body1, codexInstructions); const result2: any = await runTransform(body2, codexInstructions); - expect(result1.prompt_cache_key).toBe('meta-conv-789::fork::fork-x'); - expect(result2.prompt_cache_key).toBe('meta-conv-789::fork::fork-x'); + expect(result1.prompt_cache_key).toBe('cache_meta-conv-789-fork-fork-x'); + expect(result2.prompt_cache_key).toBe('cache_meta-conv-789-fork-fork-x'); }); - it('generates fallback prompt_cache_key when no identifiers exist', async () => { + + + it('generates deterministic fallback prompt_cache_key when no identifiers exist', async () => { const body: RequestBody = { model: 'gpt-5', input: [], }; - const result: any = await runTransform(body, codexInstructions); - expect(typeof result.prompt_cache_key).toBe('string'); - expect(result.prompt_cache_key).toMatch(/^cache_/); + const result1: any = await runTransform(body, codexInstructions); + const result2: any = await runTransform(body, codexInstructions); + expect(typeof result1.prompt_cache_key).toBe('string'); + expect(result1.prompt_cache_key).toMatch(/^cache_[a-f0-9]{12}$/); + expect(result2.prompt_cache_key).toBe(result1.prompt_cache_key); }); it('should set required Codex fields', async () => {