diff --git a/src/constants/prompts.doingTasks.test.ts b/src/constants/prompts.doingTasks.test.ts new file mode 100644 index 000000000..052efbbfe --- /dev/null +++ b/src/constants/prompts.doingTasks.test.ts @@ -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') +}) diff --git a/src/constants/prompts.ts b/src/constants/prompts.ts index 574a2ccd6..578e25269 100644 --- a/src/constants/prompts.ts +++ b/src/constants/prompts.ts @@ -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%) diff --git a/src/memdir/paths.test.ts b/src/memdir/paths.test.ts index 0adee9bdc..6d5e5216d 100644 --- a/src/memdir/paths.test.ts +++ b/src/memdir/paths.test.ts @@ -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 = {} +let _originalInteractive = false type SourceFixture = { source: string; settings: Record } 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 } } }, diff --git a/src/memdir/paths.ts b/src/memdir/paths.ts index 7ff1dd8e2..82fa50307 100644 --- a/src/memdir/paths.ts +++ b/src/memdir/paths.ts @@ -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 } diff --git a/src/query.conversationArc.test.ts b/src/query.conversationArc.test.ts index 9668763c1..a500bd95e 100644 --- a/src/query.conversationArc.test.ts +++ b/src/query.conversationArc.test.ts @@ -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') }) diff --git a/src/services/api/codexShim.test.ts b/src/services/api/codexShim.test.ts index 6eaaf9f54..e6f100d9a 100644 --- a/src/services/api/codexShim.test.ts +++ b/src/services/api/codexShim.test.ts @@ -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) } diff --git a/src/services/api/codexShim.ts b/src/services/api/codexShim.ts index 2b1e9e2aa..2b1fdebea 100644 --- a/src/services/api/codexShim.ts +++ b/src/services/api/codexShim.ts @@ -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 { - 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 = { model: options.request.resolvedModel, input: input.length > 0 diff --git a/src/services/api/openaiShim.compression.test.ts b/src/services/api/openaiShim.compression.test.ts index 52eda437a..492a40189 100644 --- a/src/services/api/openaiShim.compression.test.ts +++ b/src/services/api/openaiShim.compression.test.ts @@ -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 // ============================================================================ diff --git a/src/services/api/openaiShim/requestPreparation.ts b/src/services/api/openaiShim/requestPreparation.ts index dd497fdae..7f266a594 100644 --- a/src/services/api/openaiShim/requestPreparation.ts +++ b/src/services/api/openaiShim/requestPreparation.ts @@ -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', diff --git a/src/utils/conversationArc.test.ts b/src/utils/conversationArc.test.ts index 2980030d0..c3f6fa568 100644 --- a/src/utils/conversationArc.test.ts +++ b/src/utils/conversationArc.test.ts @@ -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 + } + }) }) }) diff --git a/src/utils/conversationArc.ts b/src/utils/conversationArc.ts index e6a38fcdf..05da1aad0 100644 --- a/src/utils/conversationArc.ts +++ b/src/utils/conversationArc.ts @@ -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++ diff --git a/src/utils/gitSettings.test.ts b/src/utils/gitSettings.test.ts new file mode 100644 index 000000000..07ae574ed --- /dev/null +++ b/src/utils/gitSettings.test.ts @@ -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 = {} + +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) +}) diff --git a/src/utils/gitSettings.ts b/src/utils/gitSettings.ts index f14299503..7ee9b3329 100644 --- a/src/utils/gitSettings.ts +++ b/src/utils/gitSettings.ts @@ -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 }