mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
perf: stop busting the prompt cache and slim per-turn context (#2142)
* perf: stop busting the prompt cache and slim per-turn context Benchmarked against a comparable harness on grok-4.6 (identical one-shot coding task), OpenClaude used 84k total tokens per task with near-zero cache reuse. Root causes and fixes: - Auto-memory now defaults off in non-interactive (-p) sessions (src/memdir/paths.ts). The conversation-arc append gated behind it rewrote the system prompt every request (Date.now()-relative durations, running token counters, per-turn RAG retrieval), which invalidates implicit prefix caches from byte one on chat-completions providers. An explicit settings opt-in (autoMemoryEnabled / memory.autoWrite) still enables it; also drops the ~3.2k-token memory protocol section from one-shot runs. - The OpenAI shim no longer runs compressToolHistory for providers with implicit prefix caching (OpenAI, xAI, DeepSeek, Kimi/Moonshot, Codex) (requestPreparation.ts, both call sites). Its end-relative window retro-edits already-sent tool results each turn, mutating the middle of the request prefix — the native Anthropic transport already guards against exactly this (claude.ts shouldCompressNativeToolHistory). - Remove the wall-clock-relative "Ns ago" line from the multi-turn tracking block (conversationArc.ts) — it changed on every request. - Ship the ~1.7k-token git commit/PR protocol in the Bash tool description only when the session is inside a git repository (gitSettings.ts). The probe is cached per cwd, not per process, since worktree tools and daemon/SDK processes change directories mid-life. - Add a code-robustness bullet to the Doing-tasks system prompt section: derive timing-sensitive logic from elapsed time, and wire up every element introduced (prompts.ts). Measured on the same benchmark (8 runs): 84k -> ~46.6k total tokens per task (-45%), 89s -> ~60s wall clock, per-call cache reads up from a constant 128 tokens to 12k-29k, baseline context 16.8k -> 11.7k tokens. Tests: 573 targeted tests pass, including new coverage for the non-interactive memory default and the per-cwd git probe; tsc --noEmit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review findings on the prompt-cache changes - Correct the prefix-caching route ids ('moonshot'/'kimi-code', not 'kimi'/'codex' — the latter never matched a real routeId) and replace the unanchored host regex with parsed-hostname comparison so path-routed gateways are not misclassified. - gitSettings: an explicit includeGitInstructions settings value now always wins over the repo probe (recourse for bare-repo/GIT_DIR layouts), and the probe reuses the existing LRU-memoized findGitRoot on getCwd() instead of a second process.cwd()-keyed implementation — fixing stale results after Bash `cd`, `git init`, and in daemon/SDK processes serving multiple directories. - Memory gate: env-provisioned memory (CLAUDE_COWORK_MEMORY_PATH_OVERRIDE, CLAUDE_CODE_REMOTE with a mounted memory dir) counts as explicit opt-in, so Cowork/remote sessions keep extraction and indexing. - Multi-turn tracking block: render only completed turns and drop the running token totals — the in-progress turn's tool-call list and the aggregate counters changed between model requests, still rewriting the system prompt mid-turn. - Update the Kimi K3 compression test to assert the new policy (history kept uncompressed on implicit-prefix-caching hosts) and extend gitSettings tests to cover settings overrides and session-cwd tracking. 569 tests pass, tsc --noEmit clean, benchmark re-run confirms metrics hold (45.1k total tokens, 62.8s, 3 calls). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: green CI and address CodeRabbit review on the prompt-cache changes CI: the conversation-arc suites assumed the multi-turn tracking block renders with only an in-progress turn, and that auto-memory is on in the (non-interactive) test process. Both now seed a completed prior turn, mark the session interactive where they exercise interactive behavior, and assert the new cache-stability invariants directly: the in-progress turn is never rendered, and no Duration/token-total lines appear. CodeRabbit findings: - Codex transport now skips tool-history compression too: Codex talks to OpenAI Responses backends with implicit prefix caching, and the end-relative compression window rewrites already-sent tool results, busting the cache (mirrors the openaiShim/requestPreparation skip). Its compression test now pins the uncompressed behavior. - New regression tests for both compression decision paths: an implicit-prefix-caching host skips compression on chat-completions and Responses requests, while a non-caching custom endpoint still compresses. - New byte-stability test: the same turn rendered twice with an advanced clock and a grown in-progress tool-call list produces an identical system prompt. - gitSettings: cover the CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS=0 defined-falsy override outside a repository. - Focused system-prompt test asserting the new timing/wiring guidance without snapshotting the full prompt. bun run check (smoke, deadcode, full suite) passes; tsc --noEmit clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: isolate CLAUDE_CODE_SIMPLE in the doing-tasks prompt test getSystemPrompt short-circuits to a minimal prompt when CLAUDE_CODE_SIMPLE is truthy; save, unset, and restore it around the test so the full prompt path is always exercised regardless of process-level state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Kevin Codex
co-authored by
Claude Fable 5
Claude Fable 5
parent
ef1a462faf
commit
31ac8a6eca
@@ -0,0 +1,35 @@
|
||||
import { afterEach, beforeEach, expect, test } from 'bun:test'
|
||||
import { withMockMacro } from 'src/test/mockMacro.js'
|
||||
import { getSystemPrompt } from './prompts.js'
|
||||
|
||||
// getSystemPrompt returns a minimal prompt without the doing-tasks section
|
||||
// when CLAUDE_CODE_SIMPLE is truthy — unset it so the test always exercises
|
||||
// the full prompt path regardless of process-level state.
|
||||
let originalSimple: string | undefined
|
||||
|
||||
beforeEach(() => {
|
||||
originalSimple = process.env.CLAUDE_CODE_SIMPLE
|
||||
delete process.env.CLAUDE_CODE_SIMPLE
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (originalSimple === undefined) {
|
||||
delete process.env.CLAUDE_CODE_SIMPLE
|
||||
} else {
|
||||
process.env.CLAUDE_CODE_SIMPLE = originalSimple
|
||||
}
|
||||
})
|
||||
|
||||
test('coding system prompt includes the timing and wiring robustness guidance', async () => {
|
||||
const prompt = await withMockMacro(
|
||||
{ ISSUES_EXPLAINER: 'report the issue at the tracker', VERSION: '0.0.0-test' },
|
||||
async () => (await getSystemPrompt([], 'test-model')).join('\n'),
|
||||
)
|
||||
|
||||
// Focused assertions on the new "Doing tasks" guidance — not a full-prompt
|
||||
// snapshot, so unrelated prompt edits don't churn this test.
|
||||
expect(prompt).toContain(
|
||||
'derive timing-sensitive logic (animation, physics, timers) from actual elapsed time',
|
||||
)
|
||||
expect(prompt).toContain('Every element you introduce must be wired up')
|
||||
})
|
||||
@@ -226,6 +226,7 @@ function getSimpleDoingTasksSection(): string {
|
||||
`Avoid giving time estimates or predictions for how long tasks will take, whether for your own work or for users planning projects. Focus on what needs to be done, not how long it might take.`,
|
||||
`If an approach fails, diagnose why before switching tactics—read the error, check your assumptions, try a focused fix. Don't retry the identical action blindly, but don't abandon a viable approach after a single failure either. Escalate to the user with ${ASK_USER_QUESTION_TOOL_NAME} only when you're genuinely stuck after investigation, not as a first response to friction.`,
|
||||
`Be careful not to introduce security vulnerabilities such as command injection, XSS, SQL injection, and other OWASP top 10 vulnerabilities. If you notice that you wrote insecure code, immediately fix it. Prioritize writing safe, secure, and correct code.`,
|
||||
`Make behavior explicit rather than environment-dependent: derive timing-sensitive logic (animation, physics, timers) from actual elapsed time instead of assuming a fixed frame or tick rate. Every element you introduce must be wired up — a UI element, state variable, or parameter that nothing ever updates or reads is a bug, not a placeholder.`,
|
||||
...codeStyleSubitems,
|
||||
`Avoid backwards-compatibility hacks like renaming unused _vars, re-exporting types, adding // removed comments for removed code, etc. If you are certain that something is unused, you can delete it completely.`,
|
||||
// @[MODEL LAUNCH]: False-claims mitigation for Capybara v8 (29-30% FC rate vs v4's 16.7%)
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import { afterEach, beforeEach, expect, test, mock } from 'bun:test'
|
||||
import { setAllowedSettingSources } from '../bootstrap/state.js'
|
||||
import {
|
||||
getIsInteractive,
|
||||
setAllowedSettingSources,
|
||||
setIsInteractive,
|
||||
} from '../bootstrap/state.js'
|
||||
import { SETTING_SOURCES } from '../utils/settings/constants.js'
|
||||
import { isAutoMemoryEnabled } from './paths.ts'
|
||||
|
||||
@@ -15,6 +19,7 @@ const realSettings = (await import(
|
||||
// opt-out can't be silently re-enabled by a narrower scope flipping the key.
|
||||
|
||||
let _originalEnv: Record<string, string | undefined> = {}
|
||||
let _originalInteractive = false
|
||||
|
||||
type SourceFixture = { source: string; settings: Record<string, unknown> }
|
||||
let _sources: SourceFixture[] = []
|
||||
@@ -41,6 +46,10 @@ beforeEach(() => {
|
||||
delete process.env.CLAUDE_CODE_REMOTE_MEMORY_DIR
|
||||
|
||||
_sources = []
|
||||
// Auto-memory defaults off for non-interactive (-p) sessions; these tests
|
||||
// exercise the interactive default unless a test overrides this.
|
||||
_originalInteractive = getIsInteractive()
|
||||
setIsInteractive(true)
|
||||
// Enable every source so getEnabledSettingSources() returns the full set in
|
||||
// priority order; the fixtures decide which of them carry a value.
|
||||
setAllowedSettingSources([...SETTING_SOURCES])
|
||||
@@ -61,6 +70,7 @@ afterEach(() => {
|
||||
process.env[k] = v
|
||||
}
|
||||
}
|
||||
setIsInteractive(_originalInteractive)
|
||||
setAllowedSettingSources([...SETTING_SOURCES])
|
||||
// mock.restore() undoes spies but NOT mock.module() registrations, which
|
||||
// otherwise leak into later test files in the same (serial) run. Re-register
|
||||
@@ -74,6 +84,20 @@ test('defaults to enabled when no source sets the key and no env override', () =
|
||||
expect(isAutoMemoryEnabled()).toBe(true)
|
||||
})
|
||||
|
||||
test('defaults to disabled in non-interactive (-p) sessions', () => {
|
||||
setIsInteractive(false)
|
||||
mockSources([{ source: 'userSettings', settings: {} }])
|
||||
expect(isAutoMemoryEnabled()).toBe(false)
|
||||
})
|
||||
|
||||
test('an explicit settings opt-in overrides the non-interactive default', () => {
|
||||
setIsInteractive(false)
|
||||
mockSources([
|
||||
{ source: 'userSettings', settings: { memory: { autoWrite: true } } },
|
||||
])
|
||||
expect(isAutoMemoryEnabled()).toBe(true)
|
||||
})
|
||||
|
||||
test('memory.autoWrite: false opts out via the new discoverable alias (#1326)', () => {
|
||||
mockSources([
|
||||
{ source: 'projectSettings', settings: { memory: { autoWrite: false } } },
|
||||
|
||||
@@ -57,6 +57,7 @@ export function isAutoMemoryEnabled(): boolean {
|
||||
// a parent-scope opt-out cannot be re-enabled by a narrower scope (#1326).
|
||||
// Per-source reads are cached (getSettingsForSource), so this stays cheap on
|
||||
// the hot path.
|
||||
let explicitOptIn = false
|
||||
for (const source of getEnabledSettingSources()) {
|
||||
const sourceSettings = getSettingsForSource(source)
|
||||
if (
|
||||
@@ -65,6 +66,26 @@ export function isAutoMemoryEnabled(): boolean {
|
||||
) {
|
||||
return false
|
||||
}
|
||||
if (
|
||||
sourceSettings?.autoMemoryEnabled === true ||
|
||||
sourceSettings?.memory?.autoWrite === true
|
||||
) {
|
||||
explicitOptIn = true
|
||||
}
|
||||
}
|
||||
// One-shot non-interactive (-p) runs have no future session to build memory
|
||||
// for: default off to skip the ~3.2k-token memory protocol section, the
|
||||
// per-request arc/RAG system-prompt append (which busts the prompt cache),
|
||||
// and turn-end extraction forks. Still enabled by any explicit provisioning:
|
||||
// a settings opt-in, CLAUDE_CODE_DISABLE_AUTO_MEMORY=0 (handled above), a
|
||||
// Cowork memory-path override, or a mounted remote memory dir — those
|
||||
// sessions are non-interactive but deliberately memory-backed.
|
||||
const envProvisionedMemory =
|
||||
hasAutoMemPathOverride() ||
|
||||
(isEnvTruthy(process.env.CLAUDE_CODE_REMOTE) &&
|
||||
Boolean(process.env.CLAUDE_CODE_REMOTE_MEMORY_DIR))
|
||||
if (!explicitOptIn && !envProvisionedMemory && getIsNonInteractiveSession()) {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -16,6 +16,11 @@ import {
|
||||
updateGoalStatus,
|
||||
} from './utils/conversationArc.js'
|
||||
import { resetGlobalGraph } from './utils/knowledgeGraph.js'
|
||||
import {
|
||||
addToolCallToTurn,
|
||||
resetMultiTurnState,
|
||||
startNewTurn,
|
||||
} from './utils/multiTurnContext.js'
|
||||
import { setClaudeConfigHomeDirForTesting } from './utils/envUtils.js'
|
||||
import { getAutoMemPath } from './memdir/paths.js'
|
||||
import { setGovernancePolicySettingsForSourceForTesting } from './utils/governancePolicy.js'
|
||||
@@ -38,11 +43,13 @@ beforeEach(async () => {
|
||||
memory: { requireApprovalBeforeWrite: false },
|
||||
}))
|
||||
resetArc()
|
||||
resetMultiTurnState()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
try {
|
||||
resetArc()
|
||||
resetMultiTurnState()
|
||||
resetGlobalGraph()
|
||||
setGovernancePolicySettingsForSourceForTesting(null)
|
||||
setClaudeConfigHomeDirForTesting(undefined)
|
||||
@@ -109,6 +116,18 @@ productionArcTest('query appends arc memory to the model system prompt without m
|
||||
updateGoalStatus(goal.id, 'completed')
|
||||
await finalizeArcTurn()
|
||||
|
||||
// Seed a prior COMPLETED turn: the multi-turn tracking block renders only
|
||||
// completed turns (the in-progress turn's tool-call list grows between
|
||||
// model requests and would bust the prompt cache). query() starts a fresh
|
||||
// turn, which completes this one.
|
||||
startNewTurn()
|
||||
addToolCallToTurn({
|
||||
id: 'call_prior',
|
||||
name: 'read_file',
|
||||
input: { path: '/prior.ts' },
|
||||
timestamp: Date.now(),
|
||||
})
|
||||
|
||||
const userMessage = createUserMessage({ content: 'review query integration' })
|
||||
let observedSystemPrompt: readonly string[] = []
|
||||
const deps: QueryDeps = {
|
||||
@@ -141,5 +160,10 @@ productionArcTest('query appends arc memory to the model system prompt without m
|
||||
expect(prompt).toContain('PERSISTENT PROJECT MEMORY')
|
||||
expect(prompt).toContain('Ship query integration')
|
||||
expect(prompt).toContain('MULTI-TURN CONTEXT TRACKING')
|
||||
expect(prompt).toContain('read_file')
|
||||
// No per-request-varying content: wall-clock durations and running token
|
||||
// totals would rewrite the system prompt every request and bust the cache.
|
||||
expect(prompt).not.toContain('Duration:')
|
||||
expect(prompt).not.toContain('Total Tokens:')
|
||||
expect(userMessage.message.content).toBe('review query integration')
|
||||
})
|
||||
|
||||
@@ -782,7 +782,11 @@ describe('Codex request translation', () => {
|
||||
expect(output?.output).toBe('first block\nsecond block')
|
||||
})
|
||||
|
||||
test('compresses structured tool results with the Codex Responses separator', async () => {
|
||||
test('keeps tool history uncompressed on the Codex transport (implicit prefix caching)', async () => {
|
||||
// Codex talks to OpenAI Responses backends, which do implicit prefix
|
||||
// caching: compressToolHistory's end-relative window would rewrite
|
||||
// already-sent tool results each turn and bust the cache, so the Codex
|
||||
// path must send history verbatim even with compression enabled.
|
||||
setToolHistoryCompressionEnabledOverrideForTest(true)
|
||||
try {
|
||||
mock.restore()
|
||||
@@ -828,8 +832,12 @@ describe('Codex request translation', () => {
|
||||
const outputs = (body?.input as Array<{ type?: string; output?: string }>)
|
||||
.filter(item => item.type === 'function_call_output')
|
||||
expect(outputs[16]?.output).toBe(
|
||||
`${'a'.repeat(1_000)}\n${'b'.repeat(999)}\n[…truncated 501 chars from tool history]`,
|
||||
`${'a'.repeat(1_000)}\n${'b'.repeat(1_500)}`,
|
||||
)
|
||||
for (const item of outputs) {
|
||||
expect(item.output).not.toContain('truncated')
|
||||
expect(item.output).not.toContain('chars omitted')
|
||||
}
|
||||
} finally {
|
||||
setToolHistoryCompressionEnabledOverrideForTest(undefined)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { APIError } from '@anthropic-ai/sdk'
|
||||
import { buildAnthropicUsageFromRawUsage } from './cacheMetrics.js'
|
||||
import { compressToolHistory } from './compressToolHistory.js'
|
||||
import { fetchWithProxyRetry } from './fetchWithProxyRetry.js'
|
||||
import { stableStringifyJson } from '../../utils/stableStringify.js'
|
||||
import type {
|
||||
@@ -578,18 +577,20 @@ export async function performCodexRequest(options: {
|
||||
signal?: AbortSignal
|
||||
fetcher?: typeof fetchWithProxyRetry
|
||||
}): Promise<Response> {
|
||||
const compressedMessages = compressToolHistory(
|
||||
options.params.messages as Array<{
|
||||
role?: string
|
||||
message?: { role?: string; content?: unknown }
|
||||
content?: unknown
|
||||
}>,
|
||||
options.request.resolvedModel,
|
||||
// Codex Responses flattens structured tool-result text with a single
|
||||
// newline, unlike Chat's double-newline message serialization.
|
||||
{ textBlockSeparator: '\n' },
|
||||
)
|
||||
const input = convertAnthropicMessagesToResponsesInput(compressedMessages)
|
||||
// No tool-history compression on the Codex transport: Codex talks to
|
||||
// OpenAI Responses backends, which do implicit prefix caching, and
|
||||
// compressToolHistory's end-relative window rewrites already-sent tool
|
||||
// results each turn — mutating the request prefix and forfeiting the
|
||||
// cache, which costs more than the compression saves. This mirrors the
|
||||
// prefix-caching skip in openaiShim/requestPreparation.ts; context
|
||||
// pressure is handled by the compaction machinery, as on the native
|
||||
// Anthropic transport.
|
||||
const rawMessages = options.params.messages as Array<{
|
||||
role?: string
|
||||
message?: { role?: string; content?: unknown }
|
||||
content?: unknown
|
||||
}>
|
||||
const input = convertAnthropicMessagesToResponsesInput(rawMessages)
|
||||
const body: Record<string, unknown> = {
|
||||
model: options.request.resolvedModel,
|
||||
input: input.length > 0
|
||||
|
||||
@@ -485,7 +485,7 @@ test('FIX: 1M context model with 30 exchanges → only first 5 mid-truncated', a
|
||||
}
|
||||
})
|
||||
|
||||
test('Kimi K3 256K selection uses its own compression window while sending the k3 API name', async () => {
|
||||
test('Kimi K3 256K selection keeps history uncompressed (implicit prefix caching) while sending the k3 API name', async () => {
|
||||
mockState.enabled = true
|
||||
process.env.OPENAI_BASE_URL = 'https://api.kimi.com/coding/v1'
|
||||
const messages = buildLongConversation(50, 5_000)
|
||||
@@ -497,9 +497,65 @@ test('Kimi K3 256K selection uses its own compression window while sending the k
|
||||
|
||||
expect(body.model).toBe('k3')
|
||||
expect(toolMessages).toHaveLength(50)
|
||||
// api.kimi.com does implicit prefix caching: compressToolHistory's
|
||||
// end-relative window would rewrite already-sent tool results each turn
|
||||
// and bust the cache, so compression is skipped for this host.
|
||||
for (const m of toolMessages) {
|
||||
expect(m.content).not.toContain('chars omitted')
|
||||
expect(m.content).not.toContain('[…truncated')
|
||||
}
|
||||
})
|
||||
|
||||
test('implicit-prefix-caching host (api.deepseek.com) skips tool-history compression', async () => {
|
||||
mockState.enabled = true
|
||||
mockState.effectiveWindow = 100_000 // small window: would compress on a custom endpoint
|
||||
process.env.OPENAI_BASE_URL = 'https://api.deepseek.com/v1'
|
||||
const messages = buildLongConversation(30, 5_000)
|
||||
|
||||
const body = await captureRequestBody(messages, 'deepseek-chat')
|
||||
const toolMessages = getToolMessages(body)
|
||||
|
||||
expect(toolMessages).toHaveLength(30)
|
||||
for (const m of toolMessages) {
|
||||
expect(m.content.length).toBe(5_000)
|
||||
expect(m.content).not.toContain('chars omitted')
|
||||
expect(m.content).not.toContain('[…truncated')
|
||||
}
|
||||
})
|
||||
|
||||
test('non-caching custom endpoint still compresses tool history', async () => {
|
||||
// Guard against the inverse regression: the prefix-caching skip must not
|
||||
// disable compression for endpoints with no implicit caching. The default
|
||||
// harness base URL (http://example.test/v1) is such an endpoint.
|
||||
mockState.enabled = true
|
||||
mockState.effectiveWindow = 100_000 // recent=12, mid=25
|
||||
const messages = buildLongConversation(30, 5_000)
|
||||
|
||||
const body = await captureRequestBody(messages, 'gpt-4o')
|
||||
const toolMessages = getToolMessages(body)
|
||||
|
||||
expect(toolMessages).toHaveLength(30)
|
||||
expect(toolMessages[0].content).toContain('chars omitted')
|
||||
})
|
||||
|
||||
test('implicit-prefix-caching host skips compression on the Responses path too', async () => {
|
||||
mockState.enabled = true
|
||||
mockState.effectiveWindow = 100_000
|
||||
const messages = buildLongConversation(30, 5_000)
|
||||
|
||||
const body = await captureResponsesRequestBody(messages, 'gpt-4o', {
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
})
|
||||
const outputs = (body.input as Array<{ type?: string; output?: string }>)
|
||||
.filter(item => item.type === 'function_call_output')
|
||||
|
||||
expect(outputs).toHaveLength(30)
|
||||
for (const item of outputs) {
|
||||
expect(item.output).not.toContain('chars omitted')
|
||||
expect(item.output).not.toContain('[…truncated')
|
||||
}
|
||||
})
|
||||
|
||||
// ============================================================================
|
||||
// FIX: stub preserves tool name and args — model can re-invoke if needed
|
||||
// ============================================================================
|
||||
|
||||
@@ -69,6 +69,46 @@ type Dependencies = {
|
||||
): boolean
|
||||
}
|
||||
|
||||
// Providers documented to do implicit prefix caching on OpenAI-compatible
|
||||
// endpoints (see the stableStringifyJson rationale in utils/stableStringify.ts:
|
||||
// OpenAI, Kimi/Moonshot, DeepSeek — plus xAI). For these, a stable request
|
||||
// prefix is worth more than tool-history compression.
|
||||
const PREFIX_CACHING_ROUTE_IDS = new Set([
|
||||
'openai',
|
||||
'xai',
|
||||
'deepseek',
|
||||
'moonshot',
|
||||
'kimi-code',
|
||||
])
|
||||
const PREFIX_CACHING_HOSTNAMES = new Set([
|
||||
'api.openai.com',
|
||||
'api.x.ai',
|
||||
'api.deepseek.com',
|
||||
'api.moonshot.ai',
|
||||
'api.moonshot.cn',
|
||||
'api.kimi.com',
|
||||
])
|
||||
|
||||
function providerUsesImplicitPrefixCaching(
|
||||
routeId: string | null | undefined,
|
||||
baseUrl: string | undefined,
|
||||
): boolean {
|
||||
if (routeId && PREFIX_CACHING_ROUTE_IDS.has(routeId)) {
|
||||
return true
|
||||
}
|
||||
if (!baseUrl) {
|
||||
return false
|
||||
}
|
||||
// Parse and compare hostnames rather than substring-matching the raw URL —
|
||||
// a path-routed gateway like https://proxy.example/api.openai.com/v1 is not
|
||||
// the provider itself and gets no implicit caching.
|
||||
try {
|
||||
return PREFIX_CACHING_HOSTNAMES.has(new URL(baseUrl).hostname.toLowerCase())
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export function prepareOpenAIRequest({
|
||||
request,
|
||||
params,
|
||||
@@ -114,11 +154,21 @@ export function prepareOpenAIRequest({
|
||||
: shimConfig.endpointPath?.startsWith('/models/gemini-')
|
||||
? 'gemini'
|
||||
: request.transport
|
||||
// Mirror the native-transport guard (shouldCompressNativeToolHistory):
|
||||
// compressToolHistory's window is measured from the end of the conversation,
|
||||
// so each turn rewrites tool results that were already sent verbatim. On
|
||||
// providers with implicit prefix caching that mutates the middle of the
|
||||
// request prefix and forfeits the entire cache downstream — costing far more
|
||||
// than the compression saves.
|
||||
const skipCompressionForPrefixCache = providerUsesImplicitPrefixCaching(
|
||||
runtimeShimContext.routeId,
|
||||
request.baseUrl,
|
||||
)
|
||||
const compressedMessages =
|
||||
effectiveTransport === 'chat_completions' ||
|
||||
effectiveTransport === 'responses' ||
|
||||
effectiveTransport === 'responses_compat'
|
||||
? fastPath.skipToolHistoryCompression
|
||||
? fastPath.skipToolHistoryCompression || skipCompressionForPrefixCache
|
||||
? rawMessages
|
||||
: compressToolHistory(rawMessages, runtimeModel, {
|
||||
textBlockSeparator:
|
||||
@@ -301,7 +351,7 @@ export function prepareOpenAIRequest({
|
||||
let responsesMessages: typeof compressedMessages | undefined
|
||||
const getResponsesInput = () => {
|
||||
responsesMessages ??= effectiveTransport === 'chat_completions'
|
||||
? fastPath.skipToolHistoryCompression
|
||||
? fastPath.skipToolHistoryCompression || skipCompressionForPrefixCache
|
||||
? rawMessages
|
||||
: compressToolHistory(rawMessages, request.resolvedModel, {
|
||||
textBlockSeparator: '\n',
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
} from './conversationArc.js'
|
||||
import { resetGlobalGraph } from './knowledgeGraph.js'
|
||||
import { setClaudeConfigHomeDirForTesting } from './envUtils.js'
|
||||
import { getIsInteractive, setIsInteractive } from '../bootstrap/state.js'
|
||||
import {
|
||||
acquireSharedMutationLock,
|
||||
releaseSharedMutationLock,
|
||||
@@ -39,6 +40,7 @@ const ARC_FILENAME = '.arc.json'
|
||||
describe('conversationArc', () => {
|
||||
let memDir: string
|
||||
let configDir: string | undefined
|
||||
let originalInteractive = false
|
||||
const originalConfigDir = process.env.CLAUDE_CONFIG_DIR
|
||||
|
||||
beforeEach(async () => {
|
||||
@@ -51,6 +53,10 @@ describe('conversationArc', () => {
|
||||
resetGlobalGraph()
|
||||
delete process.env.CLAUDE_CODE_DISABLE_AUTO_MEMORY
|
||||
delete process.env.CLAUDE_CODE_SIMPLE
|
||||
// Auto-memory defaults off in non-interactive sessions (which includes
|
||||
// the test process); these tests exercise interactive-session behavior.
|
||||
originalInteractive = getIsInteractive()
|
||||
setIsInteractive(true)
|
||||
// Disable memory-write approval for tests so arc persistence, fact
|
||||
// extraction, and vector-index writes behave as they do in production
|
||||
// when the policy is set to require-approval=false.
|
||||
@@ -63,6 +69,7 @@ describe('conversationArc', () => {
|
||||
try {
|
||||
resetArc()
|
||||
resetGlobalGraph()
|
||||
setIsInteractive(originalInteractive)
|
||||
setGovernancePolicySettingsForSourceForTesting(null)
|
||||
if (originalConfigDir === undefined) {
|
||||
delete process.env.CLAUDE_CONFIG_DIR
|
||||
@@ -509,12 +516,13 @@ describe('conversationArc', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('regression: appends multi-turn context information when MULTI_TURN_CONTEXT feature is enabled', async () => {
|
||||
it('regression: appends multi-turn context for completed turns when MULTI_TURN_CONTEXT feature is enabled', async () => {
|
||||
process.env.MULTI_TURN_CONTEXT = 'true'
|
||||
try {
|
||||
const { startNewTurn, addMessageToTurn, addToolCallToTurn, resetMultiTurnState } = await import('./multiTurnContext.js')
|
||||
resetMultiTurnState()
|
||||
|
||||
// First turn completes (a second turn starts), so it is renderable.
|
||||
startNewTurn()
|
||||
addMessageToTurn(createMessage('assistant', 'Running checks'))
|
||||
addToolCallToTurn({
|
||||
@@ -523,6 +531,13 @@ describe('conversationArc', () => {
|
||||
input: { path: '/test.ts' },
|
||||
timestamp: Date.now()
|
||||
})
|
||||
startNewTurn()
|
||||
addToolCallToTurn({
|
||||
id: 'call_current',
|
||||
name: 'write_file',
|
||||
input: { path: '/current.ts' },
|
||||
timestamp: Date.now()
|
||||
})
|
||||
|
||||
const autoMemDir = getAutoMemPath()
|
||||
clearArcArtifacts(autoMemDir)
|
||||
@@ -538,11 +553,64 @@ describe('conversationArc', () => {
|
||||
expect(promptWithArc.length).toBe(mockSystemPrompt.length + 1)
|
||||
const promptText = promptWithArc.join('\n')
|
||||
expect(promptText).toContain('MULTI-TURN CONTEXT TRACKING')
|
||||
expect(promptText).toContain('Total Turns: 1')
|
||||
expect(promptText).toContain('Total Turns: 2')
|
||||
expect(promptText).toContain('read_file')
|
||||
// The in-progress turn is never rendered: its tool-call list grows
|
||||
// between model requests and would bust the prompt cache.
|
||||
expect(promptText).not.toContain('write_file')
|
||||
expect(promptText).not.toContain('Duration:')
|
||||
expect(promptText).not.toContain('Total Tokens:')
|
||||
} finally {
|
||||
delete process.env.MULTI_TURN_CONTEXT
|
||||
}
|
||||
})
|
||||
|
||||
it('regression: multi-turn context block is byte-stable across model requests within a turn', async () => {
|
||||
process.env.MULTI_TURN_CONTEXT = 'true'
|
||||
const realDateNow = Date.now
|
||||
try {
|
||||
const { startNewTurn, addToolCallToTurn, resetMultiTurnState } = await import('./multiTurnContext.js')
|
||||
resetMultiTurnState()
|
||||
|
||||
startNewTurn()
|
||||
addToolCallToTurn({
|
||||
id: 'call_done',
|
||||
name: 'read_file',
|
||||
input: { path: '/done.ts' },
|
||||
timestamp: Date.now()
|
||||
})
|
||||
startNewTurn()
|
||||
|
||||
const autoMemDir = getAutoMemPath()
|
||||
clearArcArtifacts(autoMemDir)
|
||||
mkdirSync(autoMemDir, { recursive: true })
|
||||
initializeArc(autoMemDir)
|
||||
|
||||
const lastMessage = createMessage('user', 'continue')
|
||||
const mockSystemPrompt = ['# System Instructions']
|
||||
const { appendArcToSystemPrompt } = await import('./conversationArc.js')
|
||||
|
||||
Date.now = () => 1_000_000
|
||||
const first = await appendArcToSystemPrompt(mockSystemPrompt, [lastMessage])
|
||||
|
||||
// A later model request in the SAME turn: the clock has advanced and
|
||||
// the in-progress turn has picked up a tool call. The rendered system
|
||||
// prompt must be byte-identical, or the prompt cache is busted.
|
||||
Date.now = () => 9_000_000
|
||||
addToolCallToTurn({
|
||||
id: 'call_in_progress',
|
||||
name: 'write_file',
|
||||
input: { path: '/wip.ts' },
|
||||
timestamp: Date.now()
|
||||
})
|
||||
const second = await appendArcToSystemPrompt(mockSystemPrompt, [lastMessage])
|
||||
|
||||
expect(second.join('\n')).toBe(first.join('\n'))
|
||||
expect(first.join('\n')).not.toContain('Duration:')
|
||||
} finally {
|
||||
Date.now = realDateNow
|
||||
delete process.env.MULTI_TURN_CONTEXT
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -647,14 +647,21 @@ export async function appendArcToSystemPrompt(
|
||||
|
||||
let multiTurnContent = ''
|
||||
if (feature('MULTI_TURN_CONTEXT') || (typeof process !== 'undefined' && process.env.MULTI_TURN_CONTEXT === 'true')) {
|
||||
const { getMultiTurnStats, getRecentTurns } = await import('./multiTurnContext.js')
|
||||
const { getMultiTurnStats, getRecentTurns, getCurrentTurn } = await import('./multiTurnContext.js')
|
||||
const stats = getMultiTurnStats()
|
||||
if (stats.totalTurns > 0) {
|
||||
// Render only COMPLETED turns and no running token totals: the current
|
||||
// turn's tool-call list grows with every model request, and the
|
||||
// aggregate token counters change per request too. This block lands in
|
||||
// the system prompt, so any per-request variation rewrites the prefix
|
||||
// and busts the prompt cache. Total Turns changes once per turn, which
|
||||
// keeps the prompt stable across the requests within a turn.
|
||||
const currentTurn = getCurrentTurn()
|
||||
const recent = getRecentTurns(4)
|
||||
.filter(turn => turn !== currentTurn)
|
||||
.slice(-3)
|
||||
if (stats.totalTurns > 0 && recent.length > 0) {
|
||||
multiTurnContent = '\n--- BEGIN MULTI-TURN CONTEXT TRACKING ---\n'
|
||||
+ `Total Turns: ${stats.totalTurns}\n`
|
||||
+ `Total Tokens: ${stats.totalTokens}\n`
|
||||
+ `Average Tokens Per Turn: ${stats.avgTokensPerTurn}\n`
|
||||
const recent = getRecentTurns(3)
|
||||
const MAX_TOOL_INPUT_BYTES = 2000
|
||||
const MAX_AGGREGATE_BYTES = 10000
|
||||
let trimmedTurns = 0
|
||||
@@ -667,8 +674,10 @@ export async function appendArcToSystemPrompt(
|
||||
: redacted
|
||||
return `${tc.name}(${truncated})`
|
||||
}).join(', ') || 'None'
|
||||
// No wall-clock-relative values here: "Ns ago" changes on every
|
||||
// request, which rewrites the system prompt and busts the prompt
|
||||
// cache upstream of the entire message history.
|
||||
const turnStr = `- Turn ID: ${turn.turnId}\n`
|
||||
+ ` Duration: ${Math.round((Date.now() - turn.startTime) / 1000)}s ago\n`
|
||||
+ ` Tool Calls: ${toolCallsStr}\n`
|
||||
if (Buffer.byteLength(multiTurnContent, 'utf8') + Buffer.byteLength(turnStr, 'utf8') > MAX_AGGREGATE_BYTES) {
|
||||
trimmedTurns++
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
import { afterEach, beforeEach, expect, mock, test } from 'bun:test'
|
||||
import { mkdirSync, mkdtempSync, rmSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { getCwdState, setCwdState } from '../bootstrap/state.js'
|
||||
import { shouldIncludeGitInstructions } from './gitSettings.js'
|
||||
|
||||
const realSettings = (await import(
|
||||
`./settings/settings.js?gitSettingsTestReal=${Date.now()}-${Math.random()}`
|
||||
)) as typeof import('./settings/settings.js')
|
||||
|
||||
let originalCwdState: string
|
||||
let originalEnv: string | undefined
|
||||
let tempRoot: string
|
||||
let settingsFixture: Record<string, unknown> = {}
|
||||
|
||||
beforeEach(() => {
|
||||
originalCwdState = getCwdState()
|
||||
originalEnv = process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS
|
||||
delete process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS
|
||||
tempRoot = mkdtempSync(join(tmpdir(), 'gitsettings-test-'))
|
||||
settingsFixture = {}
|
||||
mock.module('./settings/settings.js', () => ({
|
||||
...realSettings,
|
||||
getInitialSettings: () => settingsFixture,
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
setCwdState(originalCwdState)
|
||||
if (originalEnv === undefined) {
|
||||
delete process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS
|
||||
} else {
|
||||
process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS = originalEnv
|
||||
}
|
||||
rmSync(tempRoot, { recursive: true, force: true })
|
||||
mock.module('./settings/settings.js', () => ({ ...realSettings }))
|
||||
mock.restore()
|
||||
})
|
||||
|
||||
test('omits git instructions outside a git repository', () => {
|
||||
const plainDir = join(tempRoot, 'plain')
|
||||
mkdirSync(plainDir)
|
||||
setCwdState(plainDir)
|
||||
expect(shouldIncludeGitInstructions()).toBe(false)
|
||||
})
|
||||
|
||||
test('includes git instructions inside a git repository', () => {
|
||||
const repoDir = join(tempRoot, 'repo')
|
||||
mkdirSync(join(repoDir, '.git'), { recursive: true })
|
||||
setCwdState(repoDir)
|
||||
expect(shouldIncludeGitInstructions()).toBe(true)
|
||||
})
|
||||
|
||||
test('includes git instructions in a subdirectory of a git repository', () => {
|
||||
const repoDir = join(tempRoot, 'repo2')
|
||||
mkdirSync(join(repoDir, '.git'), { recursive: true })
|
||||
const subDir = join(repoDir, 'src', 'nested')
|
||||
mkdirSync(subDir, { recursive: true })
|
||||
setCwdState(subDir)
|
||||
expect(shouldIncludeGitInstructions()).toBe(true)
|
||||
})
|
||||
|
||||
test('follows the session cwd, not the process cwd (Bash cd / daemon safety)', () => {
|
||||
const plainDir = join(tempRoot, 'plain-then-repo')
|
||||
mkdirSync(plainDir)
|
||||
const repoDir = join(tempRoot, 'repo3')
|
||||
mkdirSync(join(repoDir, '.git'), { recursive: true })
|
||||
|
||||
setCwdState(plainDir)
|
||||
expect(shouldIncludeGitInstructions()).toBe(false)
|
||||
setCwdState(repoDir)
|
||||
expect(shouldIncludeGitInstructions()).toBe(true)
|
||||
})
|
||||
|
||||
test('explicit includeGitInstructions: true wins over the repo probe', () => {
|
||||
const plainDir = join(tempRoot, 'plain-forced-on')
|
||||
mkdirSync(plainDir)
|
||||
setCwdState(plainDir)
|
||||
settingsFixture = { includeGitInstructions: true }
|
||||
expect(shouldIncludeGitInstructions()).toBe(true)
|
||||
})
|
||||
|
||||
test('explicit includeGitInstructions: false wins inside a repository', () => {
|
||||
const repoDir = join(tempRoot, 'repo-forced-off')
|
||||
mkdirSync(join(repoDir, '.git'), { recursive: true })
|
||||
setCwdState(repoDir)
|
||||
settingsFixture = { includeGitInstructions: false }
|
||||
expect(shouldIncludeGitInstructions()).toBe(false)
|
||||
})
|
||||
|
||||
test('env kill switch wins even inside a git repository', () => {
|
||||
const repoDir = join(tempRoot, 'repo4')
|
||||
mkdirSync(join(repoDir, '.git'), { recursive: true })
|
||||
setCwdState(repoDir)
|
||||
process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS = '1'
|
||||
expect(shouldIncludeGitInstructions()).toBe(false)
|
||||
})
|
||||
|
||||
test('explicit CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS=0 forces instructions on outside a repository', () => {
|
||||
const plainDir = join(tempRoot, 'plain-env-forced-on')
|
||||
mkdirSync(plainDir)
|
||||
setCwdState(plainDir)
|
||||
process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS = '0'
|
||||
// The defined-falsy env value short-circuits before repository detection.
|
||||
expect(shouldIncludeGitInstructions()).toBe(true)
|
||||
})
|
||||
@@ -6,13 +6,26 @@
|
||||
// settings.ts → git/gitignore.ts → git.ts, so git.ts → settings.ts loops.
|
||||
//
|
||||
// If you're tempted to add `import settings` to git.ts — don't. Put it here.
|
||||
// (Importing git.ts from here is fine — the dependency only flows one way.)
|
||||
|
||||
import { getCwd } from './cwd.js'
|
||||
import { isEnvDefinedFalsy, isEnvTruthy } from './envUtils.js'
|
||||
import { findGitRoot } from './git.js'
|
||||
import { getInitialSettings } from './settings/settings.js'
|
||||
|
||||
export function shouldIncludeGitInstructions(): boolean {
|
||||
const envVal = process.env.CLAUDE_CODE_DISABLE_GIT_INSTRUCTIONS
|
||||
if (isEnvTruthy(envVal)) return false
|
||||
if (isEnvDefinedFalsy(envVal)) return true
|
||||
return getInitialSettings().includeGitInstructions ?? true
|
||||
// An explicit settings value always wins — it is the recourse for layouts
|
||||
// the repo probe can't see (bare-repo checkouts with GIT_DIR/GIT_WORK_TREE,
|
||||
// --separate-git-dir outside the cwd ancestry).
|
||||
const configured = getInitialSettings().includeGitInstructions
|
||||
if (configured !== undefined) return configured
|
||||
// Default: ship the ~1.7k-token commit/PR protocol in the Bash tool
|
||||
// description only when the session is inside a git repository — it is
|
||||
// pure waste elsewhere. findGitRoot handles worktree/submodule .git files
|
||||
// and is LRU-memoized; getCwd() tracks the session cwd (Bash `cd`,
|
||||
// daemon/SDK sessions), which process.cwd() does not.
|
||||
return findGitRoot(getCwd()) !== null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user