mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
Fix snip metadata leaks and TUI corruption (#1600)
* Fix snip metadata leaks and TUI corruption Replace visible snip ID tags with internal snip_id metadata and update SnipTool guidance/tests so the model can request pruning without echoing user-visible IDs. Add compatibility parsing for legacy [id:...] markers, clarify synthetic OpenAI shim tool-result messages, and tighten test cleanup around env/session state. Mitigate Konsole+tmux rendering corruption by disabling the scroll fast path in that environment while preserving bottom-follow behavior and clearing culled cached output. Validation: bun run build; bun run smoke; ANTHROPIC_API_KEY=test-key bun run check; ANTHROPIC_API_KEY=test-key bun run test:full; python -m pytest -q python/tests; bun run security:pr-scan -- --base upstream/main. * Stabilize cooldown smoke test Use usage-bearing high-context fixtures in autoCompactCooldown tests so the cooldown assertions do not depend on process-global threshold overrides surviving full-suite order. Add CodeRabbit-requested SAST suppression comments for snip regex literals. * Use rule-id semgrep suppressions Update the snip regex suppressions to CodeRabbit's requested nosemgrep rule-id form, with the explanatory text kept in a separate comment. * Pin cooldown test context window Set and restore CLAUDE_CODE_AUTO_COMPACT_WINDOW in autoCompactCooldown tests so the high-context fixture lands above the auto-compact threshold but below the hard prompt limit on both local Windows and Linux CI. * Honor autocompact breaker metadata Block oversized requests when autocompact reports an active or tripped breaker, even if a later auto-compact config read is stale. This keeps cooldown protection active and stabilizes the CI smoke path.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, expect, mock, test } from 'bun:test'
|
||||
import { getGlobalConfig, saveGlobalConfig } from 'src/utils/config.js'
|
||||
import type { GlobalConfig } from 'src/utils/config.js'
|
||||
import {
|
||||
acquireSharedMutationLock,
|
||||
releaseSharedMutationLock,
|
||||
@@ -25,7 +25,7 @@ const REVIEW_WORKFLOW_PATH =
|
||||
|
||||
const execCalls: ExecCall[] = []
|
||||
const openedUrls: string[] = []
|
||||
let initialSetupCount: number | undefined
|
||||
let setupConfig: GlobalConfig
|
||||
let realModules: RealModules | undefined
|
||||
|
||||
function execResult(stdout = '', code = 0, stderr = '') {
|
||||
@@ -167,6 +167,12 @@ function handleGhCommand(args: string[]) {
|
||||
}
|
||||
|
||||
function installMocks(real: RealModules): void {
|
||||
mock.module('src/utils/config.js', () => ({
|
||||
saveGlobalConfig: mock((updater: (current: GlobalConfig) => GlobalConfig) => {
|
||||
setupConfig = updater(setupConfig)
|
||||
}),
|
||||
}))
|
||||
|
||||
mock.module('../../utils/execFileNoThrow.js', () => ({
|
||||
...real.execFileNoThrow,
|
||||
execFileNoThrow: mock(async (file: string, args: string[]) => {
|
||||
@@ -198,17 +204,12 @@ beforeEach(async () => {
|
||||
)
|
||||
execCalls.length = 0
|
||||
openedUrls.length = 0
|
||||
initialSetupCount = getGlobalConfig().githubActionSetupCount
|
||||
setupConfig = { numStartups: 0, githubActionSetupCount: 0 } as GlobalConfig
|
||||
installMocks(await importRealModules())
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
try {
|
||||
saveGlobalConfig(current => ({
|
||||
...current,
|
||||
githubActionSetupCount: initialSetupCount,
|
||||
}))
|
||||
initialSetupCount = undefined
|
||||
mock.restore()
|
||||
if (realModules) {
|
||||
mock.module(
|
||||
@@ -246,9 +247,7 @@ test('setupGitHubActions creates only the selected review workflow', async () =>
|
||||
expect(openedUrls[0]).toContain(
|
||||
'https://github.com/owner/repo/compare/main...add-claude-github-actions-',
|
||||
)
|
||||
expect(getGlobalConfig().githubActionSetupCount).toBe(
|
||||
(initialSetupCount ?? 0) + 1,
|
||||
)
|
||||
expect(setupConfig.githubActionSetupCount).toBe(1)
|
||||
})
|
||||
|
||||
test('setupGitHubActions skip mode configures the secret without workflow writes', async () => {
|
||||
@@ -276,7 +275,5 @@ test('setupGitHubActions skip mode configures the secret without workflow writes
|
||||
expect(openedUrls).toHaveLength(0)
|
||||
expect(secretSet?.args).toContain('CLAUDE_CODE_OAUTH_TOKEN')
|
||||
expect(secretSet?.args).toContain('oauth-token')
|
||||
expect(getGlobalConfig().githubActionSetupCount).toBe(
|
||||
(initialSetupCount ?? 0) + 1,
|
||||
)
|
||||
expect(setupConfig.githubActionSetupCount).toBe(1)
|
||||
})
|
||||
|
||||
@@ -49,6 +49,18 @@ export function didLayoutShift(): boolean {
|
||||
export type ScrollHint = { top: number; bottom: number; delta: number }
|
||||
let scrollHint: ScrollHint | null = null
|
||||
|
||||
function shouldDisableScrollFastPath(): boolean {
|
||||
if (!process.env.TMUX) return false
|
||||
if (process.env.OPENCLAUDE_KONSOLE_TMUX_FAST_SCROLL) return false
|
||||
return (
|
||||
process.env.TERM_PROGRAM === 'konsole' ||
|
||||
process.env.KONSOLE_VERSION !== undefined ||
|
||||
process.env.KONSOLE_DBUS_SESSION !== undefined
|
||||
)
|
||||
}
|
||||
|
||||
const DISABLE_SCROLL_FAST_PATH = shouldDisableScrollFastPath()
|
||||
|
||||
// Rects of position:absolute nodes from the PREVIOUS frame, used by
|
||||
// ScrollBox's blit+shift third-pass repair (see usage site). Recorded at
|
||||
// three paths — full-render nodeCache.set, node-level blit early-return,
|
||||
@@ -780,13 +792,16 @@ function renderNodeToOutput(
|
||||
const sticky =
|
||||
node.stickyScroll ?? Boolean(node.attributes['stickyScroll'])
|
||||
const prevMaxScroll = Math.max(0, prevScrollHeight - prevInnerHeight)
|
||||
// Positional check only valid when content grew — virtualization can
|
||||
// transiently SHRINK scrollHeight (tail unmount + stale heightCache
|
||||
// spacer) making scrollTop >= prevMaxScroll true by artifact, not
|
||||
// because the user was at bottom.
|
||||
const grew = scrollHeight >= prevScrollHeight
|
||||
const atBottom =
|
||||
sticky || (grew && scrollTopBeforeFollow >= prevMaxScroll)
|
||||
// Growth follow must compare against the previous max: a user who was
|
||||
// exactly at bottom before streaming adds a row is now below the new
|
||||
// maxScroll, but should still follow. When content shrinks, use the
|
||||
// current maxScroll to avoid treating virtualization height artifacts
|
||||
// as a real "was at bottom" signal.
|
||||
const positionallyAtBottom = grew
|
||||
? scrollTopBeforeFollow >= prevMaxScroll
|
||||
: scrollTopBeforeFollow >= maxScroll
|
||||
const atBottom = sticky || positionallyAtBottom
|
||||
if (atBottom && (node.pendingScrollDelta ?? 0) >= 0) {
|
||||
node.scrollTop = maxScroll
|
||||
node.pendingScrollDelta = undefined
|
||||
@@ -802,7 +817,7 @@ function renderNodeToOutput(
|
||||
// direct scrollTop writes (e.g. the alt-screen-perf test).
|
||||
if (
|
||||
node.stickyScroll === false &&
|
||||
scrollTopBeforeFollow >= prevMaxScroll
|
||||
positionallyAtBottom
|
||||
) {
|
||||
node.stickyScroll = true
|
||||
}
|
||||
@@ -930,8 +945,9 @@ function renderNodeToOutput(
|
||||
const heightDelta = scrollHeight - prevHeight
|
||||
const safeForFastPath =
|
||||
!hint ||
|
||||
heightDelta === 0 ||
|
||||
(hint.delta > 0 && heightDelta === hint.delta)
|
||||
(!DISABLE_SCROLL_FAST_PATH &&
|
||||
(heightDelta === 0 ||
|
||||
(hint.delta > 0 && heightDelta === hint.delta)))
|
||||
// scrollHint is set above when hint is captured. If safeForFastPath
|
||||
// is false the full path renders a next.screen that doesn't match
|
||||
// the DECSTBM shift — emitting DECSTBM leaves stale rows (seen as
|
||||
@@ -1469,7 +1485,27 @@ function renderScrolledChildren(
|
||||
// the subtree so when this child re-enters it doesn't fire clears
|
||||
// at positions now occupied by siblings. The viewport-clear on
|
||||
// scroll-change handles the visible-area repaint.
|
||||
if (!preserveCulledCache) dropSubtreeCache(childElem)
|
||||
//
|
||||
// IMPORTANT: before dropping cache, emit a clear at the child's
|
||||
// last known screen position. Without this, when a culled child
|
||||
// later re-enters the viewport without a full viewport clear
|
||||
// (scrolled=false), the prevScreen blit carries stale content
|
||||
// that neither viewport-clear nor position-change clear removes,
|
||||
// and ghost characters persist on the terminal indefinitely.
|
||||
// See: renderScrolledChildren's scrolled=false path.
|
||||
if (!preserveCulledCache) {
|
||||
if (cached) {
|
||||
// Defensive Math.floor: cached coords may be fractional
|
||||
// if a prior rounding path was missed; ensure integer positions.
|
||||
output.clear({
|
||||
x: Math.floor(cached.x),
|
||||
y: Math.floor(cached.y),
|
||||
width: Math.floor(cached.width),
|
||||
height: Math.floor(cached.height),
|
||||
})
|
||||
}
|
||||
dropSubtreeCache(childElem)
|
||||
}
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
+9
-2
@@ -7,6 +7,7 @@ import type { CanUseToolFn } from './hooks/useCanUseTool.js'
|
||||
import { FallbackTriggeredError } from './services/api/withRetry.js'
|
||||
import {
|
||||
calculateTokenWarningState,
|
||||
getAutoCompactThreshold,
|
||||
isAutoCompactEnabled,
|
||||
MAX_CONSECUTIVE_AUTOCOMPACT_FAILURES,
|
||||
type AutoCompactTrackingState,
|
||||
@@ -821,7 +822,9 @@ async function* queryLoop(
|
||||
tracking?.consecutiveFailures !== undefined &&
|
||||
tracking.consecutiveFailures >=
|
||||
MAX_CONSECUTIVE_AUTOCOMPACT_FAILURES &&
|
||||
isAutoCompactEnabled()
|
||||
(isAutoCompactEnabled() ||
|
||||
circuitBreakerActive === true ||
|
||||
circuitBreakerTripped === true)
|
||||
) {
|
||||
const model = toolUseContext.options.mainLoopModel
|
||||
const tokenUsage = tokenCountWithEstimation(messagesForQuery) - snipTokensFreed
|
||||
@@ -829,7 +832,11 @@ async function* queryLoop(
|
||||
tokenUsage,
|
||||
model,
|
||||
)
|
||||
if (isAboveAutoCompactThreshold) {
|
||||
const isAboveBreakerThreshold =
|
||||
isAboveAutoCompactThreshold ||
|
||||
((circuitBreakerActive === true || circuitBreakerTripped === true) &&
|
||||
tokenUsage >= getAutoCompactThreshold(model))
|
||||
if (isAboveBreakerThreshold) {
|
||||
const nowMs = Date.now()
|
||||
const retryDelayMs =
|
||||
tracking.nextRetryAtMs !== undefined
|
||||
|
||||
@@ -17,6 +17,8 @@ import { getGlobalConfig, saveGlobalConfig } from '../utils/config.js'
|
||||
|
||||
const SAVED_ENV = {
|
||||
CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR,
|
||||
CLAUDE_CODE_AUTO_COMPACT_WINDOW:
|
||||
process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW,
|
||||
CLAUDE_AUTOCOMPACT_PCT_OVERRIDE:
|
||||
process.env.CLAUDE_AUTOCOMPACT_PCT_OVERRIDE,
|
||||
DISABLE_AUTO_COMPACT: process.env.DISABLE_AUTO_COMPACT,
|
||||
@@ -31,6 +33,7 @@ beforeEach(async () => {
|
||||
process.env.CLAUDE_CONFIG_DIR = tempDir
|
||||
savedAutoCompactEnabled = getGlobalConfig().autoCompactEnabled
|
||||
saveGlobalConfig(current => ({ ...current, autoCompactEnabled: true }))
|
||||
process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW = '200000'
|
||||
process.env.CLAUDE_AUTOCOMPACT_PCT_OVERRIDE = '1'
|
||||
delete process.env.DISABLE_AUTO_COMPACT
|
||||
delete process.env.DISABLE_COMPACT
|
||||
@@ -72,6 +75,28 @@ function userMessage(content: string): Message {
|
||||
}
|
||||
}
|
||||
|
||||
function highContextMessages(): Message[] {
|
||||
return [
|
||||
{
|
||||
type: 'assistant',
|
||||
message: {
|
||||
id: 'msg-high-context',
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: 'previous response' }],
|
||||
usage: {
|
||||
input_tokens: 170_000,
|
||||
output_tokens: 1_000,
|
||||
cache_creation_input_tokens: 0,
|
||||
cache_read_input_tokens: 0,
|
||||
},
|
||||
},
|
||||
uuid: `assistant-${Math.random()}` as Message['uuid'],
|
||||
timestamp: new Date().toISOString(),
|
||||
} as unknown as Message,
|
||||
userMessage('continue'),
|
||||
]
|
||||
}
|
||||
|
||||
function toolUseContext() {
|
||||
const abortController = new AbortController()
|
||||
return {
|
||||
@@ -142,7 +167,7 @@ async function drain<T, TReturn>(
|
||||
}
|
||||
|
||||
test('active auto-compact cooldown blocks before model call with cooldown guidance', async () => {
|
||||
const messages = [userMessage('x'.repeat(100_000))]
|
||||
const messages = highContextMessages()
|
||||
const nextRetryAtMs = Date.now() + 60_000
|
||||
const callModel = mock(() => {
|
||||
throw new Error('model should not be called while autocompact cools down')
|
||||
@@ -197,7 +222,7 @@ test('active auto-compact cooldown blocks before model call with cooldown guidan
|
||||
})
|
||||
|
||||
test('auto-compact cooldown tracking is carried into the next query call', async () => {
|
||||
const messages = [userMessage('x'.repeat(100_000))]
|
||||
const messages = highContextMessages()
|
||||
const nextRetryAtMs = Date.now() + 60_000
|
||||
const seenTracking: Array<AutoCompactTrackingState | undefined> = []
|
||||
const callModel = mock(() => {
|
||||
@@ -340,7 +365,7 @@ test('breaker metadata tracking callback publishes a fresh object', async () =>
|
||||
|
||||
const { terminal } = await drain(
|
||||
query({
|
||||
messages: [userMessage('x'.repeat(100_000))],
|
||||
messages: highContextMessages(),
|
||||
systemPrompt: asSystemPrompt([]),
|
||||
userContext: {},
|
||||
systemContext: {},
|
||||
|
||||
@@ -5242,7 +5242,7 @@ test('preserves valid tool_result and drops orphan tool_result', async () => {
|
||||
// 2. User content ("What happened?") -> role 'user'
|
||||
// This triggers the tool -> assistant injection.
|
||||
const assistantMessages = messages.filter(m => m.role === 'assistant')
|
||||
expect(assistantMessages.some(m => m.content === '[Tool execution interrupted by user]')).toBe(true)
|
||||
expect(assistantMessages.some(m => m.content === '[Tool results received]')).toBe(true)
|
||||
})
|
||||
|
||||
test('drops empty assistant message when only thinking block was present and stripped', async () => {
|
||||
@@ -5325,7 +5325,9 @@ test('injects semantic assistant message when tool result is followed by user me
|
||||
|
||||
const semanticMsg = messages[2]
|
||||
expect(semanticMsg.role).toBe('assistant')
|
||||
expect(semanticMsg.content).toBe('[Tool execution interrupted by user]')
|
||||
expect(semanticMsg.content).toBe('[Tool results received]')
|
||||
expect(semanticMsg.content).not.toContain('interrupted')
|
||||
expect(semanticMsg.content).not.toContain('user')
|
||||
})
|
||||
|
||||
test('Moonshot: uses max_tokens (not max_completion_tokens) and strips store', async () => {
|
||||
|
||||
@@ -763,13 +763,14 @@ function convertMessages(
|
||||
const prev = coalesced[coalesced.length - 1]
|
||||
|
||||
// Mistral/Devstral: 'tool' message must be followed by an 'assistant' message.
|
||||
// If a 'tool' result is followed by a 'user' message, we must inject a semantic
|
||||
// assistant response to satisfy the strict role sequence:
|
||||
// If a 'tool' result is followed by a 'user' message, inject a neutral
|
||||
// assistant boundary to satisfy the strict role sequence without implying
|
||||
// that the user interrupted or cancelled anything:
|
||||
// ... -> assistant (calls) -> tool (results) -> assistant (semantic) -> user (next)
|
||||
if (prev && prev.role === 'tool' && msg.role === 'user') {
|
||||
coalesced.push({
|
||||
role: 'assistant',
|
||||
content: '[Tool execution interrupted by user]',
|
||||
content: '[Tool results received]',
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -285,6 +285,39 @@ describe('markForSnip', () => {
|
||||
const matched = markForSnip([deriveShortMessageId(realUuid), 'xxxxxx'], messages)
|
||||
expect(matched).toEqual([realUuid])
|
||||
})
|
||||
|
||||
test('accepts legacy bracketed id syntax from older contexts', () => {
|
||||
const realUuid = 'a1b2c3d4-0000-0000-0000-0000000000bb'
|
||||
const messages = [makeUser(realUuid)]
|
||||
const matched = markForSnip(
|
||||
[`[id:${deriveShortMessageId(realUuid)}]`],
|
||||
messages,
|
||||
)
|
||||
expect(matched).toEqual([realUuid])
|
||||
})
|
||||
|
||||
test('accepts snip_id-prefixed metadata syntax', () => {
|
||||
const realUuid = 'a1b2c3d4-0000-0000-0000-0000000000cc'
|
||||
const messages = [makeUser(realUuid)]
|
||||
const matched = markForSnip(
|
||||
[`snip_id=${deriveShortMessageId(realUuid)}`],
|
||||
messages,
|
||||
)
|
||||
expect(matched).toEqual([realUuid])
|
||||
})
|
||||
|
||||
test('accepts copied system-reminder metadata syntax', () => {
|
||||
const realUuid = 'a1b2c3d4-0000-0000-0000-0000000000dd'
|
||||
const messages = [makeUser(realUuid)]
|
||||
const id = deriveShortMessageId(realUuid)
|
||||
const matched = markForSnip(
|
||||
[
|
||||
`<system-reminder>snip_id=${id}; system-generated; for snip tool use only;</system-reminder>`,
|
||||
],
|
||||
messages,
|
||||
)
|
||||
expect(matched).toEqual([realUuid])
|
||||
})
|
||||
})
|
||||
|
||||
describe('shouldNudgeForSnips', () => {
|
||||
|
||||
@@ -13,6 +13,24 @@ import { deriveShortMessageId } from '../../utils/messages.js'
|
||||
// Populated by SnipTool.call(); consumed by snipCompactIfNeeded().
|
||||
const pendingSnipUuids = new Set<UUID>()
|
||||
|
||||
function normalizeSnipShortId(shortId: string): string {
|
||||
const trimmed = shortId.trim()
|
||||
// 6 = deriveShortMessageId output length (base36)
|
||||
// Regex literal only; this does not execute user input.
|
||||
// nosemgrep: coderabbit.command-injection.exec-js
|
||||
const snipMetadataMatch = /\bsnip_id=([a-z0-9]{6})\b/i.exec(trimmed)
|
||||
if (snipMetadataMatch) {
|
||||
return snipMetadataMatch[1]!.toLowerCase()
|
||||
}
|
||||
// Regex literal only; this does not execute user input.
|
||||
// nosemgrep: coderabbit.command-injection.exec-js
|
||||
const legacyMatch = /^\[id:([a-z0-9]{6})\]$/i.exec(trimmed)
|
||||
if (legacyMatch) {
|
||||
return legacyMatch[1]!.toLowerCase()
|
||||
}
|
||||
return trimmed.toLowerCase()
|
||||
}
|
||||
|
||||
// Returns the distinct UUIDs that actually resolved against this conversation
|
||||
// and were queued. Unresolvable short IDs (stale or hallucinated) are skipped,
|
||||
// so callers can report the genuinely-queued count rather than the raw request
|
||||
@@ -26,7 +44,8 @@ export function markForSnip(shortIds: string[], messages: any[]): UUID[] {
|
||||
}
|
||||
const matched = new Set<UUID>()
|
||||
for (const shortId of shortIds) {
|
||||
const uuid = shortIdToUuid.get(shortId)
|
||||
const normalizedShortId = normalizeSnipShortId(shortId)
|
||||
const uuid = shortIdToUuid.get(normalizedShortId)
|
||||
if (uuid) {
|
||||
pendingSnipUuids.add(uuid)
|
||||
matched.add(uuid)
|
||||
@@ -41,9 +60,11 @@ export function isSnipRuntimeEnabled(): boolean {
|
||||
|
||||
export const SNIP_NUDGE_TEXT =
|
||||
`Your context window is filling up. Use the \`snip\` tool to remove messages ` +
|
||||
`that are no longer needed — look for \`[id:...]\` tags on user messages and pass the IDs ` +
|
||||
`of stale sections (old explorations, superseded plans, resolved errors). This frees up ` +
|
||||
`space so you can continue working without a full compaction.`
|
||||
`that are no longer needed — silently use system-generated \`snip_id=...\` ` +
|
||||
`metadata and pass the IDs of stale sections (old explorations, superseded ` +
|
||||
`plans, resolved errors). These ids are not user-provided content; do not ` +
|
||||
`describe or mention them. This frees up space so you can continue working ` +
|
||||
`without a full compaction.`
|
||||
|
||||
// Nudge once every ~10 000 tokens of new content since the last reset point.
|
||||
const NUDGE_INTERVAL_TOKENS = 10_000
|
||||
|
||||
@@ -33,16 +33,14 @@ describe('SnipTool.mapToolResultToToolResultBlockParam', () => {
|
||||
expect(content).not.toContain('They will be removed from context')
|
||||
})
|
||||
|
||||
test('explains the refusal condition and how to observe/repair it', () => {
|
||||
// The model needs the failure signal the prior wording omitted: a kept
|
||||
// message still carries its [id:...] tag next turn, and the fix is to snip
|
||||
// every result from that parallel-tool turn together.
|
||||
test('does not echo internal id mechanics', () => {
|
||||
const out = SnipTool.mapToolResultToToolResultBlockParam(
|
||||
{ sniped: 1 },
|
||||
'toolu_abc',
|
||||
)
|
||||
const content = String(out.content)
|
||||
expect(content).toMatch(/kept|orphan/i)
|
||||
expect(content).toContain('[id:')
|
||||
expect(content).toContain('Queued 1 message(s) for snipping')
|
||||
expect(content).not.toContain('[id:')
|
||||
expect(content).not.toMatch(/tag|snip_id|message id/i)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -10,7 +10,7 @@ const inputSchema = lazySchema(() =>
|
||||
message_ids: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'Short message IDs to remove — the [id:XXXXXX] values appended to user messages.',
|
||||
'Short internal snip message IDs to remove.',
|
||||
),
|
||||
}),
|
||||
)
|
||||
@@ -60,17 +60,10 @@ export const SnipTool = buildTool({
|
||||
return {
|
||||
type: 'tool_result',
|
||||
tool_use_id: toolUseID,
|
||||
// A snip is a queued request, not a guaranteed removal: snipCompactIfNeeded
|
||||
// refuses to drop a tool_result whose paired tool_use would survive (it
|
||||
// would orphan the tool call), so the request can no-op. Describe it
|
||||
// honestly and give the model the observable signal + repair, otherwise it
|
||||
// treats a structural no-op as a successful context reduction.
|
||||
content:
|
||||
`Queued ${content.sniped} message(s) for snipping before the next model call. ` +
|
||||
`A queued message is kept if removing it would orphan a tool call (for example, ` +
|
||||
`snipping one result from a turn that ran several tools in parallel). If a message ` +
|
||||
`you queued still shows its [id:...] tag on the next turn, it was kept; snip all of ` +
|
||||
`that turn's tool results together to remove them.`,
|
||||
`Some may be kept if removing them would split a tool call from its result; ` +
|
||||
`when pruning old tool output, queue the whole related tool interaction.`,
|
||||
}
|
||||
},
|
||||
} satisfies ToolDef<InputSchema, Output>)
|
||||
|
||||
@@ -3,7 +3,7 @@ export const SNIP_TOOL_NAME = 'snip'
|
||||
export function getPrompt(): string {
|
||||
return `Remove specific messages from your context window to free up space.
|
||||
|
||||
When your context is getting long, look for \`[id:XXXXXX]\` tags appended to user messages. Pass those IDs to this tool to queue those messages (and their associated tool calls and results) for removal before the next model call. A queued message is kept if removing it would orphan a tool call (for example, snipping one result from a turn that ran several tools in parallel); if a message you queued still shows its \`[id:...]\` tag next turn, it was kept.
|
||||
When your context is getting long, silently use system-generated \`snip_id=...\` metadata to queue messages (and their associated tool calls and results) for removal before the next model call. Pass only the raw ID value to this tool. These ids are not user-provided content: do not describe them, mention them, or say that the user provided them, including in thinking. A queued message may be kept if removing it would split a tool call from its result; when pruning old tool output, queue the whole related tool interaction or parallel-tool turn. If old output remains, do not treat it as current work unless the latest user request asks for it.
|
||||
|
||||
Good candidates to snip:
|
||||
- Old exploratory searches that led nowhere
|
||||
|
||||
@@ -78,40 +78,59 @@ describe('countMcpToolTokens', () => {
|
||||
})
|
||||
|
||||
test('keeps deferred MCP schemas excluded from the outgoing request estimate when Tool Search is deferred', async () => {
|
||||
const result = await countMcpToolTokens(
|
||||
[
|
||||
makeToolSearchTool(),
|
||||
makeMcpTool('mcp__alpha__search'),
|
||||
makeMcpTool('mcp__beta__list'),
|
||||
],
|
||||
emptyPermissionContext,
|
||||
{ activeAgents: [] } as never,
|
||||
'test-model',
|
||||
[],
|
||||
countToolDefinitions,
|
||||
)
|
||||
|
||||
expect(result.mcpToolTokens).toBe(0)
|
||||
expect(result.deferredToolTokens).toBeGreaterThan(0)
|
||||
expect(result.mcpToolDetails.every(tool => !tool.isLoaded)).toBe(true)
|
||||
|
||||
const report = createRequestSizeReport(
|
||||
makeContextData({
|
||||
categories: [
|
||||
{
|
||||
name: 'MCP tools (deferred)',
|
||||
tokens: result.deferredToolTokens,
|
||||
color: 'inactive',
|
||||
isDeferred: true,
|
||||
},
|
||||
const previousToolSearch = process.env.ENABLE_TOOL_SEARCH
|
||||
const previousDisableBetas =
|
||||
process.env.CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS
|
||||
process.env.ENABLE_TOOL_SEARCH = 'true'
|
||||
delete process.env.CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS
|
||||
try {
|
||||
const result = await countMcpToolTokens(
|
||||
[
|
||||
makeToolSearchTool(),
|
||||
makeMcpTool('mcp__alpha__search'),
|
||||
makeMcpTool('mcp__beta__list'),
|
||||
],
|
||||
mcpTools: result.mcpToolDetails,
|
||||
}),
|
||||
)
|
||||
const labels = report.contributors.map(contributor => contributor.label)
|
||||
emptyPermissionContext,
|
||||
{ activeAgents: [] } as never,
|
||||
'test-model',
|
||||
[],
|
||||
countToolDefinitions,
|
||||
)
|
||||
|
||||
expect(report.estimatedTokens).toBe(0)
|
||||
expect(labels).not.toContain('MCP server alpha')
|
||||
expect(labels).not.toContain('MCP server beta')
|
||||
expect(result.mcpToolTokens).toBe(0)
|
||||
expect(result.deferredToolTokens).toBeGreaterThan(0)
|
||||
expect(result.mcpToolDetails.every(tool => !tool.isLoaded)).toBe(true)
|
||||
|
||||
const report = createRequestSizeReport(
|
||||
makeContextData({
|
||||
categories: [
|
||||
{
|
||||
name: 'MCP tools (deferred)',
|
||||
tokens: result.deferredToolTokens,
|
||||
color: 'inactive',
|
||||
isDeferred: true,
|
||||
},
|
||||
],
|
||||
mcpTools: result.mcpToolDetails,
|
||||
}),
|
||||
)
|
||||
const labels = report.contributors.map(contributor => contributor.label)
|
||||
|
||||
expect(report.estimatedTokens).toBe(0)
|
||||
expect(labels).not.toContain('MCP server alpha')
|
||||
expect(labels).not.toContain('MCP server beta')
|
||||
} finally {
|
||||
if (previousToolSearch === undefined) {
|
||||
delete process.env.ENABLE_TOOL_SEARCH
|
||||
} else {
|
||||
process.env.ENABLE_TOOL_SEARCH = previousToolSearch
|
||||
}
|
||||
if (previousDisableBetas === undefined) {
|
||||
delete process.env.CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS
|
||||
} else {
|
||||
process.env.CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS =
|
||||
previousDisableBetas
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,23 +11,27 @@ const UUID = 'a1b2c3d4-0000-0000-0000-000000000099'
|
||||
const UUID_B = 'b2c3d4e5-0000-0000-0000-000000000088'
|
||||
|
||||
function tagFor(uuid: string): string {
|
||||
return `[id:${deriveShortMessageId(uuid)}]`
|
||||
return `snip_id=${deriveShortMessageId(uuid)}`
|
||||
}
|
||||
|
||||
function countTags(out: UserMessage): number {
|
||||
const c = out.message.content
|
||||
const s = typeof c === 'string' ? c : JSON.stringify(c)
|
||||
return (s.match(/\[id:/g) || []).length
|
||||
return (s.match(/snip_id=/g) || []).length
|
||||
}
|
||||
|
||||
describe('appendMessageTagToUserMessage', () => {
|
||||
test('appends the tag to string content', () => {
|
||||
test('appends internal snip metadata to string content', () => {
|
||||
const msg = { ...createUserMessage({ content: 'hello' }), uuid: UUID }
|
||||
const out = appendMessageTagToUserMessage(msg as UserMessage)
|
||||
expect(out.message.content).toBe(`hello\n${tagFor(UUID)}`)
|
||||
expect(out.message.content).toContain('hello')
|
||||
expect(out.message.content).toContain(tagFor(UUID))
|
||||
expect(out.message.content).toContain('do not discuss in thinking')
|
||||
expect(out.message.content).not.toContain('[id:')
|
||||
expect(out.message.content).not.toContain('user-provided')
|
||||
})
|
||||
|
||||
test('appends the tag to the last text block of array content', () => {
|
||||
test('appends internal snip metadata to the last text block of array content', () => {
|
||||
const msg = {
|
||||
...createUserMessage({
|
||||
content: [{ type: 'text', text: 'first' }],
|
||||
@@ -36,10 +40,14 @@ describe('appendMessageTagToUserMessage', () => {
|
||||
}
|
||||
const out = appendMessageTagToUserMessage(msg as UserMessage)
|
||||
const blocks = out.message.content as any[]
|
||||
expect(blocks[blocks.length - 1].text).toBe(`first\n${tagFor(UUID)}`)
|
||||
expect(blocks[blocks.length - 1].text).toContain('first')
|
||||
expect(blocks[blocks.length - 1].text).toContain(tagFor(UUID))
|
||||
expect(blocks[blocks.length - 1].text).toContain('do not discuss in thinking')
|
||||
expect(blocks[blocks.length - 1].text).not.toContain('[id:')
|
||||
expect(blocks[blocks.length - 1].text).not.toContain('user-provided')
|
||||
})
|
||||
|
||||
test('adds a visible tag to a pure tool_result message (large Read/Bash output)', () => {
|
||||
test('adds internal snip metadata to a pure tool_result message (large Read/Bash output)', () => {
|
||||
const msg = {
|
||||
...createUserMessage({
|
||||
content: [
|
||||
@@ -56,9 +64,12 @@ describe('appendMessageTagToUserMessage', () => {
|
||||
const blocks = out.message.content as any[]
|
||||
// The tool_result block is preserved so snip pairing still works.
|
||||
expect(blocks.some(b => b.type === 'tool_result')).toBe(true)
|
||||
// A visible [id:...] tag is now present for the model to reference.
|
||||
// Internal metadata is present for snip without looking user-authored.
|
||||
const flattened = JSON.stringify(blocks)
|
||||
expect(flattened).toContain(tagFor(UUID))
|
||||
expect(flattened).toContain('do not discuss in thinking')
|
||||
expect(flattened).not.toContain('[id:')
|
||||
expect(flattened).not.toContain('user-provided')
|
||||
})
|
||||
|
||||
test('leaves a meta message untouched', () => {
|
||||
@@ -137,6 +148,7 @@ describe('appendMessageTagToUserMessage', () => {
|
||||
// Both siblings' ids survive the merge, so both are snippable.
|
||||
expect(flattened).toContain(tagFor(UUID))
|
||||
expect(flattened).toContain(tagFor(UUID_B))
|
||||
expect(flattened).not.toContain('[id:')
|
||||
// Both tool_result blocks are preserved for snip pairing.
|
||||
const blocks = merged.message.content as any[]
|
||||
expect(blocks.filter(b => b.type === 'tool_result').length).toBe(2)
|
||||
|
||||
+31
-27
@@ -197,7 +197,8 @@ export function withMemoryCorrectionHint(message: string): string {
|
||||
|
||||
/**
|
||||
* Derive a short stable message ID (6-char base36 string) from a UUID.
|
||||
* Used for snip tool referencing — injected into API-bound messages as [id:...] tags.
|
||||
* Used for snip tool referencing — injected into API-bound messages as internal
|
||||
* system-reminder metadata.
|
||||
* Deterministic: same UUID always produces the same short ID.
|
||||
*/
|
||||
export function deriveShortMessageId(uuid: string): string {
|
||||
@@ -1613,7 +1614,7 @@ function stripUnavailableToolReferencesFromUserMessage(
|
||||
}
|
||||
|
||||
/**
|
||||
* Appends a [id:...] message ID tag to the last text block of a user message.
|
||||
* Appends internal snip metadata to the last text block of a user message.
|
||||
* Only mutates the API-bound copy, not the stored message.
|
||||
* This lets Claude reference message IDs when calling the snip tool.
|
||||
*/
|
||||
@@ -1624,26 +1625,28 @@ export function appendMessageTagToUserMessage(
|
||||
return message
|
||||
}
|
||||
|
||||
const idToken = `[id:${deriveShortMessageId(message.uuid)}]`
|
||||
const tag = `\n${idToken}`
|
||||
const idToken = deriveShortMessageId(message.uuid)
|
||||
const tag =
|
||||
`\n<system-reminder>snip_id=${idToken}; system-generated; ` +
|
||||
`for snip tool use only; do not discuss in thinking or responses.</system-reminder>`
|
||||
|
||||
const content = message.message.content
|
||||
|
||||
// Idempotency: normalizeMessagesForAPI re-runs over messages that are carried
|
||||
// forward as loop state (query.ts builds toolResults from this function's own
|
||||
// normalized output, then re-normalizes that state next turn). Without this
|
||||
// guard each pass stacks another [id:] tag on every prior tool result. The
|
||||
// token is derived from this message's own uuid, so its presence means we
|
||||
// already tagged it (string body, last text block, or the dedicated
|
||||
// tool_result text block all embed the bare idToken). Leave it untouched.
|
||||
// guard each pass stacks another internal marker on every prior tool result. The
|
||||
// token is derived from this message's own uuid, so its presence inside the
|
||||
// internal marker means we already tagged it (string body, last text block, or
|
||||
// the dedicated tool_result text block). Leave it untouched.
|
||||
const alreadyTagged =
|
||||
typeof content === 'string'
|
||||
? content.includes(idToken)
|
||||
? content.includes(`snip_id=${idToken}`)
|
||||
: Array.isArray(content) &&
|
||||
content.some(
|
||||
block =>
|
||||
block!.type === 'text' &&
|
||||
(block as TextBlockParam).text.includes(idToken),
|
||||
(block as TextBlockParam).text.includes(`snip_id=${idToken}`),
|
||||
)
|
||||
if (alreadyTagged) {
|
||||
return message
|
||||
@@ -1674,9 +1677,10 @@ export function appendMessageTagToUserMessage(
|
||||
}
|
||||
if (lastTextIdx === -1) {
|
||||
// Pure tool_result messages (large Read/Bash outputs) carry no text block
|
||||
// to host the tag, yet they are the highest-value snip targets. Append a
|
||||
// dedicated text block so the model can see and reference the [id:] tag.
|
||||
// The tool_result block is left intact, so snip pairing is unaffected.
|
||||
// to host the metadata, yet they are the highest-value snip targets. Append
|
||||
// a dedicated text block so the model can see the internal snip id without
|
||||
// making it look user-authored. The tool_result block is left intact, so
|
||||
// snip pairing is unaffected.
|
||||
if (!content.some(block => block!.type === 'tool_result')) {
|
||||
return message
|
||||
}
|
||||
@@ -2039,8 +2043,8 @@ export function normalizeMessagesForAPI(
|
||||
// Build set of available tool names for filtering unavailable tool references
|
||||
const availableToolNames = new Set(tools.map(t => t.name))
|
||||
|
||||
// Whether to inject [id:] snip tags this pass. Gate must match
|
||||
// SnipTool.isEnabled() and skip test mode — tags change message content
|
||||
// Whether to inject internal snip ids this pass. Gate must match
|
||||
// SnipTool.isEnabled() and skip test mode — markers change message content
|
||||
// hashes, breaking VCR fixture lookup. Computed once here so the pre-merge
|
||||
// injection (in the user case) and the post-merge sweep below share it.
|
||||
let injectSnipTags = false
|
||||
@@ -2205,8 +2209,8 @@ export function normalizeMessagesForAPI(
|
||||
// tool_reference inside the block is a server ValueError.
|
||||
// Idempotent: query.ts calls this per-tool-result; the output flows
|
||||
// back through here via claude.ts on the next API request. The first
|
||||
// pass's sibling gets a \n[id:xxx] suffix from appendMessageTag below,
|
||||
// so startsWith matches both bare and tagged forms.
|
||||
// pass's sibling gets an internal snip marker from appendMessageTag
|
||||
// below, so startsWith matches both bare and marked forms.
|
||||
//
|
||||
// Gated OFF when tengu_toolref_defer_j8m is active — that gate
|
||||
// enables relocateToolReferenceSiblings in post-processing below,
|
||||
@@ -2242,7 +2246,7 @@ export function normalizeMessagesForAPI(
|
||||
}
|
||||
}
|
||||
|
||||
// Inject the snip [id:] tag BEFORE merging consecutive user messages.
|
||||
// Inject the internal snip id BEFORE merging consecutive user messages.
|
||||
// A parallel-tool assistant turn yields several adjacent tool_result
|
||||
// user messages; mergeUserMessages keeps only the first operand's uuid,
|
||||
// so tagging only after the merge (the sweep below) would expose just
|
||||
@@ -2253,7 +2257,7 @@ export function normalizeMessagesForAPI(
|
||||
// through the merge (joinTextAtSeam keeps both text blocks) and matches
|
||||
// the live path, where each result is tagged individually at push time
|
||||
// (query.ts). appendMessageTagToUserMessage is idempotent, so the
|
||||
// post-merge sweep below is a no-op for messages already tagged here.
|
||||
// post-merge sweep below is a no-op for messages already marked here.
|
||||
if (injectSnipTags) {
|
||||
normalizedMessage = appendMessageTagToUserMessage(normalizedMessage)
|
||||
}
|
||||
@@ -2419,12 +2423,12 @@ export function normalizeMessagesForAPI(
|
||||
// image-in-error tool_result 400s forever.
|
||||
const sanitized = sanitizeErrorToolResultContent(smooshed)
|
||||
|
||||
// Post-merge sweep for snip [id:] tags. User messages folded in the loop above
|
||||
// are already tagged pre-merge (so every parallel-tool sibling's id survives
|
||||
// the merge); this catches user messages synthesized during normalization that
|
||||
// never went through that path — local_command system messages and attachments
|
||||
// promoted to user turns. appendMessageTagToUserMessage is idempotent, so it is
|
||||
// a no-op for anything already tagged above.
|
||||
// Post-merge sweep for internal snip ids. User messages folded in the loop
|
||||
// above are already marked pre-merge (so every parallel-tool sibling's id
|
||||
// survives the merge); this catches user messages synthesized during
|
||||
// normalization that never went through that path — local_command system
|
||||
// messages and attachments promoted to user turns. appendMessageTagToUserMessage
|
||||
// is idempotent, so it is a no-op for anything already marked above.
|
||||
if (injectSnipTags) {
|
||||
for (let i = 0; i < sanitized.length; i++) {
|
||||
if (sanitized[i]!.type === 'user') {
|
||||
@@ -2486,7 +2490,7 @@ export function mergeUserMessages(a: UserMessage, b: UserMessage): UserMessage {
|
||||
if (feature('HISTORY_SNIP')) {
|
||||
// A merged message is only meta if ALL merged messages are meta. If any
|
||||
// operand is real user content, the result must not be flagged isMeta
|
||||
// (so [id:] tags get injected and it's treated as user-visible content).
|
||||
// (so internal snip ids get injected and it's treated as user-visible content).
|
||||
// Gated behind the full runtime check because changing isMeta semantics
|
||||
// affects downstream callers (e.g., VCR fixture hashing in SDK harness
|
||||
// tests), so this must only fire when snip is actually enabled — not
|
||||
@@ -2510,7 +2514,7 @@ export function mergeUserMessages(a: UserMessage, b: UserMessage): UserMessage {
|
||||
}
|
||||
return {
|
||||
...a,
|
||||
// Preserve the non-meta message's uuid so [id:] tags (derived from uuid)
|
||||
// Preserve the non-meta message's uuid so snip ids (derived from uuid)
|
||||
// stay stable across API calls (meta messages like system context get fresh uuids each call)
|
||||
uuid: a.isMeta ? b.uuid : a.uuid,
|
||||
message: {
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
buildConversationChain,
|
||||
loadTranscriptFile,
|
||||
recordGoalState,
|
||||
flushSessionStorage,
|
||||
resetProjectForTesting,
|
||||
resetSessionFilePointer,
|
||||
setSessionFileForTesting,
|
||||
@@ -22,7 +23,12 @@ import {
|
||||
stripPersistedToolUseResultsFromJSONLBuffer,
|
||||
} from './sessionStorage.ts'
|
||||
import { createGoalState } from '../services/goal/state.js'
|
||||
import { getSessionId, switchSession } from '../bootstrap/state.js'
|
||||
import {
|
||||
getSessionId,
|
||||
isSessionPersistenceDisabled,
|
||||
setSessionPersistenceDisabled,
|
||||
switchSession,
|
||||
} from '../bootstrap/state.js'
|
||||
import type { GoalState } from '../services/goal/types.js'
|
||||
|
||||
const tempDirs: string[] = []
|
||||
@@ -164,8 +170,16 @@ function readGoalStateEntries(text: string): Array<{ goal: GoalState | null }> {
|
||||
|
||||
async function withSessionPersistence<T>(fn: () => Promise<T>): Promise<T> {
|
||||
const originalPersistence = process.env.TEST_ENABLE_SESSION_PERSISTENCE
|
||||
const originalSessionPersistence = process.env.ENABLE_SESSION_PERSISTENCE
|
||||
const originalSkipPromptHistory = process.env.CLAUDE_CODE_SKIP_PROMPT_HISTORY
|
||||
const originalNodeEnv = process.env.NODE_ENV
|
||||
const originalSessionId = getSessionId()
|
||||
const originalSessionPersistenceDisabled = isSessionPersistenceDisabled()
|
||||
process.env.NODE_ENV = 'development'
|
||||
process.env.TEST_ENABLE_SESSION_PERSISTENCE = 'true'
|
||||
process.env.ENABLE_SESSION_PERSISTENCE = 'true'
|
||||
delete process.env.CLAUDE_CODE_SKIP_PROMPT_HISTORY
|
||||
setSessionPersistenceDisabled(false)
|
||||
try {
|
||||
resetProjectForTesting()
|
||||
return await fn()
|
||||
@@ -175,6 +189,22 @@ async function withSessionPersistence<T>(fn: () => Promise<T>): Promise<T> {
|
||||
} else {
|
||||
process.env.TEST_ENABLE_SESSION_PERSISTENCE = originalPersistence
|
||||
}
|
||||
if (originalSessionPersistence === undefined) {
|
||||
delete process.env.ENABLE_SESSION_PERSISTENCE
|
||||
} else {
|
||||
process.env.ENABLE_SESSION_PERSISTENCE = originalSessionPersistence
|
||||
}
|
||||
if (originalSkipPromptHistory === undefined) {
|
||||
delete process.env.CLAUDE_CODE_SKIP_PROMPT_HISTORY
|
||||
} else {
|
||||
process.env.CLAUDE_CODE_SKIP_PROMPT_HISTORY = originalSkipPromptHistory
|
||||
}
|
||||
if (originalNodeEnv === undefined) {
|
||||
delete process.env.NODE_ENV
|
||||
} else {
|
||||
process.env.NODE_ENV = originalNodeEnv
|
||||
}
|
||||
setSessionPersistenceDisabled(originalSessionPersistenceDisabled)
|
||||
switchSession(originalSessionId)
|
||||
resetProjectForTesting()
|
||||
}
|
||||
@@ -547,6 +577,7 @@ test('recordGoalState writes goal metadata durably before resolving', async () =
|
||||
},
|
||||
sessionId as never,
|
||||
)
|
||||
await flushSessionStorage()
|
||||
|
||||
const text = await readFile(filePath, 'utf8')
|
||||
expect(text).toContain('"type":"goal-state"')
|
||||
|
||||
Reference in New Issue
Block a user