Skip to content

fix(sdk): bound Claude provider observer context to prevent overflow (#2956)#2957

Open
dndungu wants to merge 4 commits into
thedotmack:mainfrom
dndungu:fix/2956-claude-provider-context-bound
Open

fix(sdk): bound Claude provider observer context to prevent overflow (#2956)#2957
dndungu wants to merge 4 commits into
thedotmack:mainfrom
dndungu:fix/2956-claude-provider-context-bound

Conversation

@dndungu

@dndungu dndungu commented Jun 16, 2026

Copy link
Copy Markdown

Fixes #2956.

Problem

When using the Claude provider (CLAUDE_MEM_PROVIDER=claude, ClaudeProvider / Agent SDK), the observer agent accumulates every observation into a single long-lived SDK conversation that we resume via memorySessionId. There is no cumulative cap on how large that context grows. On long primary sessions it eventually exceeds the model window, the SDK returns "Prompt is too long", and the run aborts — saving zero memory for the whole session. The OpenRouter and Gemini providers already bound their context; the Claude provider never got an equivalent. That asymmetry is the bug.

Why OpenRouter's fix doesn't port directly

OpenRouterProvider.truncateHistory slices session.conversationHistory before each stateless HTTP request because OpenRouter resends the full history every turn. In ClaudeProvider, that array is only ever pushed to, never resent — the SDK owns the real context internally. Slicing it would be a no-op. The only lever for the SDK path is to start a fresh SDK session once the context grows too large.

What's already handled (not touched)

This PR adds the missing proactive cumulative bound.

The fix

After each turn, read the SDK-reported full context size (input_tokens + cache_creation_input_tokens + cache_read_input_tokens) and, if it exceeds the new CLAUDE_MEM_CLAUDE_MAX_TOKENS cap (default 150000), call resetSessionForFreshStart (clears memorySessionId, sets forceInit) and abort — mirroring the existing in-loop quota guard. The next observation ingest starts a fresh, small SDK session. The guard fires only after the current turn's memory has been emitted, so it loses nothing, and it stops the observer before the hard overflow wall.

Changes

  • SettingsDefaultsManager: add CLAUDE_MEM_CLAUDE_MAX_TOKENS (default 150000), on par with CLAUDE_MEM_GEMINI_MAX_TOKENS / CLAUDE_MEM_OPENROUTER_MAX_TOKENS.
  • SettingsRoutes: allowlist + validate (1000..1000000, mirroring GEMINI_MAX_TOKENS).
  • ClaudeProvider: proactive context-bound guard in the startSession message loop.
  • worker-types / SessionRoutes / scrub / telemetry: add a distinct context_bound abort reason (proactive reset vs reactive overflow).

Tests

  • tests/shared/settings-defaults-manager.test.ts: asserts the new default.
  • tests/claude-provider-context-bound.test.ts: threshold predicate, full-context accounting, and the resetSessionForFreshStart effect reused by the guard.

bun test (provider + settings + telemetry + session-route suites) green; tsc --noEmit clean; npm run build succeeds.

dndungu added 2 commits June 16, 2026 12:50
…hedotmack#2956)

The Claude provider (ClaudeProvider / Agent SDK) had no cumulative context
bound. Observations accumulate in a single long-lived SDK conversation that we
resume via memorySessionId, so on long primary sessions the context eventually
exceeds the model window, the SDK returns "Prompt is too long", and the run
aborts saving zero memory. The OpenRouter and Gemini providers already cap their
context; the Claude provider never got an equivalent.

OpenRouter's truncateHistory cannot be ported directly: it slices
session.conversationHistory before each stateless request, but in ClaudeProvider
that array is only ever pushed to (the SDK owns the real context), so slicing it
is a no-op. The only lever is to start a fresh SDK session once the context
grows too large.

Add a proactive token bound mirroring the existing in-loop quota guard: after
each turn, read the SDK-reported full context size (input + cache-creation +
cache-read tokens) and, if it exceeds the new CLAUDE_MEM_CLAUDE_MAX_TOKENS cap
(default 150000), call resetSessionForFreshStart (clears memorySessionId, sets
forceInit) and abort. The next observation ingest then starts a fresh, small SDK
session. The guard fires only after the current turn's memory is emitted, so it
loses nothing, and it stops the observer before the hard overflow wall.

- SettingsDefaultsManager: add CLAUDE_MEM_CLAUDE_MAX_TOKENS (default 150000)
- SettingsRoutes: allowlist + validate (1000..1000000, mirrors GEMINI_MAX_TOKENS)
- ClaudeProvider: proactive context-bound guard in the message loop
- worker-types / SessionRoutes / scrub / telemetry: add 'context_bound' abort reason

Per-observation payload trimming (prompts.ts) and reactive overflow handling
already existed; this adds the missing proactive cumulative bound.
- settings-defaults-manager: assert CLAUDE_MEM_CLAUDE_MAX_TOKENS default
- claude-provider-context-bound: threshold predicate, full-context accounting,
  and resetSessionForFreshStart effect reused by the proactive guard
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a proactive cumulative context bound to the Claude (Agent SDK) provider, closing an asymmetry where OpenRouter and Gemini already capped their history but the SDK path had no equivalent guard. After each assistant turn the new guard reads the full SDK-reported context size (input_tokens + cache_creation_input_tokens + cache_read_input_tokens), and if it exceeds CLAUDE_MEM_CLAUDE_MAX_TOKENS (default 150 k) it calls resetSessionForFreshStart and breaks the loop — saving the current turn's memory before the hard model-window wall would zero everything out.

  • New exported guard helpers (resolveObserverMaxTokens, computeFullContextTokens, observerContextExceeded) are placed as module-level functions in ClaudeProvider.ts and exercised directly by the new test suite, ensuring predicate drift is caught.
  • Settings plumbing (SettingsDefaultsManager, SettingsRoutes) adds CLAUDE_MEM_CLAUDE_MAX_TOKENS with validation matching the Gemini equivalent (1 000–1 000 000, default 150 000).
  • Telemetry (worker-types, SessionRoutes, scrub, telemetry) introduces a distinct context_bound abort reason so dashboards can distinguish proactive resets from reactive overflow aborts.

Confidence Score: 5/5

The change is purely additive: it wires a proactive guard that fires only after the current turn's memory is already saved, so the worst-case outcome of the new code path is a slightly early session reset — not lost data or a hard crash.

All core logic (token accumulation, reset, abort) follows the existing quota-guard pattern precisely; the three exported helper functions are simple arithmetic with no side effects; settings validation mirrors the established Gemini equivalent; and the new test suite exercises the real exported predicates rather than inline copies. No data-loss or overflow scenario is introduced by this change.

The only item worth a second look is the resolveObserverMaxTokens helper in ClaudeProvider.ts — the || fallback is consistent with how the rest of the codebase handles default resolution but leaves a gap for manually-edited negative values.

Important Files Changed

Filename Overview
src/services/worker/ClaudeProvider.ts Adds three exported guard helpers and the proactive context-bound check inside the assistant-message branch of the SDK for-await loop; logic is sound, scoping is correct, and the reset/break mirrors the existing quota guard pattern.
src/shared/SettingsDefaultsManager.ts Adds CLAUDE_MEM_CLAUDE_MAX_TOKENS interface field and default value (150000) symmetric with GEMINI/OPENROUTER equivalents; straightforward.
src/services/worker/http/routes/SettingsRoutes.ts Allowlists and validates CLAUDE_MEM_CLAUDE_MAX_TOKENS (1000–1000000), mirroring the Gemini validation block; correct.
src/services/worker/http/routes/SessionRoutes.ts Adds context-bound → context_bound mapping in normalizeAbortReason, consistent with the existing restart-guard → restart_guard pattern.
src/services/worker-types.ts Adds 'context-bound' to the abortReason union; documents the new internal reason alongside existing hyphenated values.
tests/claude-provider-context-bound.test.ts New test suite exercises cap resolution, threshold predicate, full-context accounting, and resetSessionForFreshStart through the real exported functions; covers edge cases (NaN, empty, exact-at-boundary).
tests/shared/settings-defaults-manager.test.ts Adds assertion for CLAUDE_MEM_CLAUDE_MAX_TOKENS default value; straightforward.
src/services/telemetry/scrub.ts Updates scrub allowlist comment to include context_bound in the abort_reason enum documentation; no logic changes.
src/npx-cli/commands/telemetry.ts Adds context_bound to the user-facing abort_reason field description in the telemetry command output; documentation only.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[startSession called] --> B[resolveObserverMaxTokens\nfrom settings]
    B --> C[for await message of queryResult]
    C --> D{message.type?}
    D -- system/rate_limit --> E[quota guard\nexisting]
    D -- assistant --> F[let currentContextTokens = 0]
    D -- result --> G[telemetry / cost capture\nexisting]
    F --> H{text includes\n'Prompt is too long'?}
    H -- yes --> I[resetSessionForFreshStart\nabortReason = 'overflow'\nreturn]
    H -- no --> J[if usage:\ncomputeFullContextTokens\ncurrentContextTokens = fullContextTokens]
    J --> K[processAgentResponse\nsave memory]
    K --> L{observerContextExceeded\ncurrentContextTokens > maxObserverTokens?}
    L -- no --> C
    L -- yes --> M[resetSessionForFreshStart\nabortReason = 'context-bound'\nabortController.abort\nbreak — NEW]
    E --> N{abort?}
    N -- yes --> O[break]
    N -- no --> C
    M --> P[finally: flush events\nensureSdkProcessExit]
    O --> P
    G --> C
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[startSession called] --> B[resolveObserverMaxTokens\nfrom settings]
    B --> C[for await message of queryResult]
    C --> D{message.type?}
    D -- system/rate_limit --> E[quota guard\nexisting]
    D -- assistant --> F[let currentContextTokens = 0]
    D -- result --> G[telemetry / cost capture\nexisting]
    F --> H{text includes\n'Prompt is too long'?}
    H -- yes --> I[resetSessionForFreshStart\nabortReason = 'overflow'\nreturn]
    H -- no --> J[if usage:\ncomputeFullContextTokens\ncurrentContextTokens = fullContextTokens]
    J --> K[processAgentResponse\nsave memory]
    K --> L{observerContextExceeded\ncurrentContextTokens > maxObserverTokens?}
    L -- no --> C
    L -- yes --> M[resetSessionForFreshStart\nabortReason = 'context-bound'\nabortController.abort\nbreak — NEW]
    E --> N{abort?}
    N -- yes --> O[break]
    N -- no --> C
    M --> P[finally: flush events\nensureSdkProcessExit]
    O --> P
    G --> C
Loading

Reviews (2): Last reviewed commit: "test(sdk): exercise real guard helpers f..." | Re-trigger Greptile

Comment thread tests/claude-provider-context-bound.test.ts Outdated
Comment thread src/services/worker-types.ts Outdated
dndungu added 2 commits June 16, 2026 15:25
…pers, hyphenated abort reason

Greptile review feedback on thedotmack#2956:
- Export resolveObserverMaxTokens / computeFullContextTokens /
  observerContextExceeded from ClaudeProvider and use them in the guard, so the
  proactive-bound tests exercise the real logic instead of reimplementing it
  (a > vs >= drift would now fail a test).
- Rename the internal abort reason 'context_bound' -> 'context-bound' to match
  the existing hyphenated convention ('restart-guard'); normalizeAbortReason maps
  it to the underscore telemetry enum 'context_bound', mirroring restart_guard.
…2957 review)

Import resolveObserverMaxTokens / computeFullContextTokens /
observerContextExceeded from ClaudeProvider instead of mirroring them, and add a
cap-resolution suite, so guard drift is detectable.
@dndungu

dndungu commented Jun 16, 2026

Copy link
Copy Markdown
Author

Thanks for the review. Both points addressed in ce1b06b:

1. Tests now exercise the real guard logic. Extracted the guard's three decision units into exported functions on ClaudeProviderresolveObserverMaxTokens, computeFullContextTokens, and observerContextExceeded — and the guard itself now calls them. The tests import these instead of reimplementing them, so a drift like >>= or dropping cache_creation_input_tokens from the sum would now fail a test rather than pass silently. Added a cap-resolution suite too.

2. Abort-reason naming aligned. Renamed the internal value 'context_bound''context-bound' to match the existing hyphenated convention ('restart-guard'). normalizeAbortReason maps it to the underscore telemetry enum context_bound, exactly as it does restart-guardrestart_guard, keeping the internal-hyphen / external-underscore boundary consistent.

tsc --noEmit clean; context-bound + settings + telemetry + session-route suites green.

@dndungu

dndungu commented Jun 25, 2026

Copy link
Copy Markdown
Author

Could I please get a review here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude provider (SDKAgent) has no context bound — observer overflows with "Prompt is too long" and saves zero memory on long sessions

1 participant