fix: bug-batch — 17 issues + 4 foundations (chroma, opencode, parser, OAuth, paths, uptime, classification) (#2282)
* feat: foundations F1-F4 + simple bug fixes Foundations (no consumer adoption yet): - F1 spawnHidden wrapper at src/shared/spawn.ts - F2 paths namespace with 18 accessors + invariant test (tests/shared/paths.test.ts) - F3 getUptimeSeconds at src/shared/uptime.ts - F4 ClassifiedProviderError at src/services/worker/provider-errors.ts + 6 tests Issue fixes (file-isolated, parallel-safe): - #2231: SECURITY.md at repo root for GitHub Security tab - #2240: dedupe observationIds before Chroma sync (ResponseProcessor.ts) - #2247: add task_complete to Codex session-end events - #2243: rsync excludes scripts/package.json + scripts/node_modules Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: validate Claude executable with --version and detect desktop app Extract findClaudeExecutable() into shared utility used by both SDKAgent and KnowledgeAgent (deduplication). Every candidate is now validated with --version (3s timeout). Desktop app executables in AppData/Program Files get an actionable error message directing users to install the CLI via npm. Closes #2222 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use Zod schemas in OpenCode plugin to fix _zod.def crash OpenCode 1.14.x walks arg._zod.def at plugin registration, which crashes on plain JSON Schema objects like {type: "string"}. Replace with z.string().describe() so the Zod internals are present. Closes #2226, #2225, #2154 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: neutralize chroma-mcp CPU storm at the root Two surgical fixes to the chroma backfill path that together cause the sustained 60–80% CPU + orphan accumulation pattern reported across 1. ChromaMcpManager.getSpawnEnv: cap embedding-thread fanout ONNX Runtime / OpenBLAS / MKL all default to cpu_count(), so a 12-core machine spins 12 threads burning embeddings concurrently. The user's getSpawnEnv only handled SSL certs — no thread limits at all. Inject OMP_NUM_THREADS / ONNX_NUM_THREADS / OPENBLAS_NUM_THREADS / MKL_NUM_THREADS defaults of 2 (only if user hasn't pinned them), and ANONYMIZED_TELEMETRY=false to stop background HTTP from the embedding subprocess. Closes the storm at the source. 2. ChromaSync.backfill{Observations,Summaries,Prompts}: per-batch watermark The bump was in a trailing finally block. SIGKILL / OOM / power loss mid-flight skips finally entirely, so the watermark stayed at 0 and the next worker boot re-embedded the entire history (16K obs in #2220's case), which then pegged CPU forever in combination with (1). Move the bump inside the loop so progress is durable per batch. Closes #2214. Verification: - 26/26 chroma tests pass (tests/services/sync, tests/integration/chroma-vector-sync) - Bundle confirms thread caps and per-batch bumps are present - Full suite: 1429 pass / 20 fail — pre-existing failures only, no regression vs v12.4.9 baseline (1429 pass / 27 fail) Closes #2214. Substantially de-amplifies #2220 (the structural Job-Object cleanup is still tracked separately at #2216). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: kill chroma-mcp process tree and limit backfill concurrency Three fixes for orphan chroma-mcp processes and resource exhaustion: 1. killProcessTree() in ChromaMcpManager.stop() tears down the full uvx->uv->python->chroma-mcp spawn chain (pkill -P on POSIX, taskkill /T on Windows) before MCP client.close(). 2. Register chroma process with pgid for supervisor shutdown cascade. 3. backfillAllProjects() now processes max 3 projects concurrently with a re-entrancy guard to prevent overlapping fire-and-forget runs. Closes #2216, advances #2220, #2213 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * build: regenerate plugin artifacts after cherry-picks Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: foundation consumers + Cursor/stdin/queue/docs fixes F1 spawnHidden adoption (#2236): - 8 spawn → spawnHidden conversions across worker-utils, ProcessManager, npx-cli (install/runtime), supervisor/process-registry F3 getUptimeSeconds adoption (#2250): - Server.ts:165 (THE BUG: returned ms) - Server.ts:270, SessionRoutes.ts:326 (4th ms-bug consumer found), DataRoutes.ts:225 (refactor for consistency) #2188 stdin '{}' fallback removal: - Diagnostic logging to <DATA_DIR>/logs/runner-errors.log + CAPTURE_BROKEN marker; exit 0 to preserve Windows Terminal exit-code strategy #2196 ANTHROPIC_BASE_URL docs: - New docs/public/configuration/custom-anthropic-backends.mdx - Note: issue may need separate auto-detect feature; docs document existing plumbing only #2242 check-pending-queue endpoints: - Point at /api/processing-status + /api/processing per DataRoutes.ts; honor CLAUDE_MEM_WORKER_PORT env #2248 Cursor sessions never summarized: - Pulled reporter wbingli's tested fix (commit 46eaba44) - Bug A: cursor adapter now derives transcriptPath from cwd+sessionId - Bug B: parser accepts both line.type and line.role - Bug C: walk backward, prefer non-empty text, fallback to empty - Tests: 10-case regression suite + tests/fixtures/cursor-session.jsonl Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: F2 paths namespace adoption (#2237 + #2238) Replaced 24 hardcoded homedir() + '.claude-mem' sites across 18 source files with paths.<accessor>() calls from src/shared/paths.ts. Accessors used: dataDir, workerPid, settings, database, chroma, combinedCerts, transcriptsConfig, transcriptsState, corpora, supervisorRegistry, envFile, logsDir. Sites converted (file:area): - src/cli/claude-md-commands.ts (database) - src/services/context/ContextConfigLoader.ts (settings) - src/services/infrastructure/ProcessManager.ts (workerPid) - src/services/infrastructure/WorktreeAdoption.ts (settings) - src/services/integrations/CodexCliInstaller.ts (settings) - src/services/sync/ChromaMcpManager.ts (chroma + combinedCerts) - src/services/transcripts/config.ts (transcriptsConfig + transcriptsState) - src/services/worker/ClaudeProvider.ts (envFile) - src/services/worker/GeminiProvider.ts (envFile + 2 more) - src/services/worker/http/routes/DataRoutes.ts (dataDir) - src/services/worker/http/routes/SettingsRoutes.ts (settings + envFile) - src/services/worker/knowledge/CorpusStore.ts (corpora) - src/shared/EnvManager.ts (envFile) - src/supervisor/index.ts (supervisorRegistry) - src/supervisor/process-registry.ts (supervisorRegistry) - src/supervisor/shutdown.ts (supervisorRegistry) - src/utils/claude-md-utils.ts (database) - src/utils/logger.ts (logsDir + settings, lazy to avoid cycle) CLAUDE_MEM_DATA_DIR override now flows through 100% of the worker runtime; no per-file env reads needed. Verification: - Grep guard: zero homedir+'.claude-mem' sites remain in src/ (excluding paths.ts itself and SettingsDefaultsManager.ts) - F2 invariant test: 3/3 pass (60 expects) - Foundation tests: 19/19 pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: F4 provider classification + parser fence + OAuth keychain F4 adoption (#2244 + #2254): - Per-provider classifiers: classifyClaudeError, classifyGeminiError, classifyOpenRouterError. Each lives in the provider file. - New retry helper at src/services/worker/retry.ts: withRetry() honors ClassifiedProviderError.kind; retriable=transient/rate_limit (with retryAfterMs); not retriable=unrecoverable/auth_invalid/quota_exhausted. maxRetries=2, perAttemptTimeout=30s, exponential backoff with jitter. - GeminiProvider + OpenRouterProvider fetch calls wrapped with retry. Best-effort request-id capture (x-goog-request-id, x-request-id, x-openrouter-request-id) for dedup logging. - Deleted unrecoverablePatterns allowlist at worker-service.ts:540 area; worker dispatches on err.kind instead. - 28 new classifier tests at tests/worker/provider-classifiers.test.ts: 429-no-Retry-After, 500-with-quota-exceeded, OverloadedError, per-provider auth_invalid signals. #2233 Part A — parser fence handling: - src/sdk/prompts.ts: removed 4 fence markers from XML example blocks. Model now sees plain XML, eliminating the failure-mode that drained quota via repeated retries. - src/sdk/parser.ts: stripCodeFences() at top, called before parseAgentXml. Fence-tolerant regardless of model behavior. - TODO comment references #2233 Part B (tool-use migration as separate scope). - 4 fence-tolerance tests added to tests/sdk/parser.test.ts. #2215 OAuth token keychain: - New src/shared/oauth-token.ts (~360 LOC): readClaudeOAuthToken() reads from platform-native credential stores at worker spawn-time. - macOS: security find-generic-password -s "Claude Code-credentials" - Windows: PowerShell wrapper around CredRead (Win32 Advapi32.dll) - Linux: secret-tool lookup - Fallback: env CLAUDE_CODE_OAUTH_TOKEN with JWT exp claim or sidecar expiresAt validation; refuses stale-token injection. - EnvManager.buildIsolatedEnvWithFreshOAuth() (async) replaces silent process.env copy. Empty injection on absent; marker write on expired. - <DATA_DIR>/oauth-stale.marker surfaces "re-login via Claude Desktop" via existing SessionStart additionalContext mechanism (context.ts). - ClaudeProvider.startSession + KnowledgeAgent.prime/executeQuery now await the async env builder. - 17 oauth-token tests covering decodeJwtExpMs, marker round-trip, env-fallback expiry detection. Verification: - npx tsc --noEmit: only pre-existing bun-types error - bun test (foundations + new): 70 pass, 0 new fails (8 fails are pre-existing parser.test.ts cases unrelated to fence work) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: #2234 quota-aware wall-clock guard New src/services/worker/RateLimitStore.ts (207 LOC) — vendor pattern from meridian/rateLimitStore.ts (MIT, copied not depended). API: - class RateLimitStore: set/get/getAll/getMostRecentByWindow/size/clear, in-memory last-write-wins keyed by rateLimitType. - globalRateLimitStore singleton. - shouldAbortForQuota(authMethod, store, now?) → {abort, reason?, window?} - isApiKeyAuth(authMethod): matches both verbose getAuthMethodDescription strings and concise "api_key". Thresholds (auth-type gated): - api_key: never aborts (user authorized per-call spend). - cli/oauth/subscription: - five_hour utilization >= 0.95 OR resetsAt within 15min (with 0.85 utilization floor to avoid false trip on freshly-reset windows) - seven_day_opus >= 0.93 - seven_day_sonnet >= 0.92 - seven_day >= 0.93 - overage >= 0.95 ClaudeProvider integration (line 198, for-await loop): - Detects message.type === 'system' && subtype === 'rate_limit' - Records rate_limit_info via globalRateLimitStore.set - Calls shouldAbortForQuota(authMethod, globalRateLimitStore) - On abort: session.abortReason = 'quota:<window>', abortController.abort, break out of loop. Worker continues other sessions. Health endpoint (Server.ts:174): - New rateLimits field on /api/health from getMostRecentByWindow(). - Field shape: {five_hour?, seven_day?, seven_day_opus?, seven_day_sonnet?, overage?} each carrying utilization, status, resetsAt, observedAt. Tests (tests/worker/rate-limit-store.test.ts): - 22 cases covering store CRUD, isApiKeyAuth, abort decision matrix. - api_key never aborts at any utilization. - cli aborts at threshold breaches per window. - Reset-grace buffer with utilization floor. Verification: - npx tsc --noEmit: only pre-existing bun error - bun test tests/worker/rate-limit-store.test.ts: 22/22 pass - bun test tests/claude-provider-resume.test.ts: 9/9 pass - bun test tests/server/: 44/44 pass Plugin artifacts regenerated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * build: regenerate worker-service.cjs after final build-and-sync Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: align test assertions with F4 classification + timeout Two test fixes for branch-introduced regressions vs main: 1. tests/gemini_provider.test.ts "should throw on other errors": F4's classifyGeminiError replaced upstream Error message with ClassifiedProviderError. Test was pinned to pre-F4 string. Updated assertion to match new "Gemini bad request (status 400)". 2. tests/infrastructure/graceful-shutdown.test.ts: Test pokes real ~/.claude-mem/supervisor.json registry which on a developer machine contains live worker + chroma-mcp PIDs. SIGTERM → wait → SIGKILL cascade takes ~6s end-to-end. Bumped per-test timeout to 15000ms. Underlying shutdown code unchanged. Future cleanup should mock getSupervisor() here. Result: branch failure count == main (77 pre-existing failures). No new regressions from this branch's work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address 4 Greptile P1/P2 findings on PR #2282 P1 (real bug): clearStaleMarker silently broken in ESM - src/shared/oauth-token.ts:14: add unlinkSync to top-level fs import - src/shared/oauth-token.ts:342: drop inline require('fs'), call unlinkSync directly. ESM has no require, so the previous code threw ReferenceError swallowed by try/catch — making clearStaleMarker a permanent no-op. Stale oauth marker would persist indefinitely after Claude Desktop refreshed the token. P2 (security): execSync shell-string interpolation - src/shared/find-claude-executable.ts:39: execSync(`"${candidate}" --version`) → execFileSync(candidate, ['--version']). Path containing ", ;, & — reachable on Windows via crafted CLAUDE_CODE_PATH in settings.json — would otherwise produce a malformed/exploitable command. P2 (security): PowerShell username injection - src/shared/oauth-token.ts:119: userInfo().username escaped with PS single-quote convention (' → '') before interpolation into `'Claude Code-credentials:${user}'`. Defensive against future Windows versions or domain-joined machines that may permit ' in usernames. P2 (style): Unreachable throw lastError post-loop - src/services/worker/retry.ts:109: explained as the safety net for opts.maxRetries < 0 (pathological input where the loop never executes and lastError is undefined). Annotated with comment + descriptive fallback Error so the dead-looking code is now self-documenting. Verification: - npx tsc --noEmit: clean (only pre-existing bun-types error) - bun test tests/shared/oauth-token.test.ts tests/worker/provider-classifiers.test.ts tests/worker/provider-errors.test.ts: 50 pass / 0 fail Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: tighten SECURITY.md data-flow and audit dates Fixes CodeRabbit comments #3178957249 (Data Storage section overstated "no external transmission" — softened to call out Claude Agent SDK, alternate provider, Chroma MCP, OAuth keychain, and registry fetches) and #3178957250 (Next Scheduled Audit was earlier than Last Updated; bumped Last Updated to 2026-05-03 and audit to 2026-09-16) on PR #2282. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: drop inline require('fs') in paths.ts Fixes CodeRabbit outside-diff comment on src/shared/paths.ts:25-29 from PR #2282 review. resolveDataDir() ran require('fs') inside an ESM module (this file uses import.meta.url and .js imports), which can break in strict ESM environments. readFileSync now imports at the top alongside existsSync/mkdirSync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: block CLAUDE_CODE_OAUTH_TOKEN from parent env (issue #2215) Fixes CodeRabbit outside-diff comment on src/shared/EnvManager.ts:14-17 from PR #2282 review. The OAuth-token leak fix was bypassed because buildIsolatedEnv() copied every parent env var that wasn't in BLOCKED_ENV_VARS, but CLAUDE_CODE_OAUTH_TOKEN was not blocked. A stale parent token therefore still reached isolatedEnv even when the fresh keychain read returned expired/absent — defeating the fix documented inline at lines 178-183. Adds CLAUDE_CODE_OAUTH_TOKEN to BLOCKED_ENV_VARS and defensively deletes it again at the top of buildIsolatedEnvWithFreshOAuth() so the fresh-spawn-time read is the only path that can populate it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: validate cursor sessionId against path traversal Fixes CodeRabbit comment #3178957252 on PR #2282. The Cursor adapter took sessionId straight from stdin and concatenated it into a join(homedir(), '.cursor', 'projects', ..., sessionId, ...) path. A crafted value containing path separators or '..' segments could escape ~/.cursor/projects, and the later transcript read would then probe arbitrary local files. deriveCursorTranscriptPath() now rejects any sessionId that doesn't match /^[A-Za-z0-9_-]+$/ — Cursor's real session ids are UUID-style identifiers, so the safe whitelist is non-disruptive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: scope stripCodeFences() to full-wrapper payloads only Fixes CodeRabbit comment #3178957253 on PR #2282. The previous regex greedily removed the first opening and last closing triple-backticks anywhere in the input, which could mangle valid content with internal fenced examples or surrounding prose — and ran before XML parsing so it created false negatives. stripCodeFences() now only strips when the entire payload is a single fenced block (start-to-end, with optional language tag and surrounding whitespace), capturing the inner content. Adds a regression test that feeds prose with internal triple-backtick markers around a real <observation> block and asserts the inner ``` are preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: honor abortSignal during retry backoff sleep Fixes CodeRabbit comment #3178957263 on PR #2282. The retry helper used an unconditional `setTimeout` Promise for backoff between attempts, so an external abort that fired during the wait was delayed until the timer completed. The backoff now races setTimeout against opts.abortSignal: if the signal flips, the timer is cleared and the Promise rejects with 'Aborted' immediately. The abort listener is registered with { once: true } and removed when the timer fires to avoid leaks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: abort immediately on provider-side rejected status Fixes CodeRabbit comment #3178957261 on PR #2282. shouldAbortForQuota() only checked utilization thresholds and reset-grace heuristics; a snapshot with status='rejected' (or overageStatus='rejected' on the overage window) but no utilization number could still return { abort: false }, letting the worker keep consuming after the provider had already declared the bucket exhausted. Provider-side rejection is now checked before utilization. When either rejection signal is present the guard returns abort=true with reason "quota:<window> rejected by provider". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: only bump Chroma watermark on confirmed batch writes Fixes CodeRabbit comments #3178957259 (watermark advances on swallowed batch failures) and #3178957260 (backfillInProgress can stick true if init throws) on PR #2282. addDocuments() previously logged and swallowed per-batch failures with a void return type, so all three backfill loops (observations, summaries, prompts) bumped the watermark unconditionally after the call — turning a transient Chroma failure into permanently-skipped records. addDocuments() now returns the count of documents that actually landed (including delete+add reconcile retries), and each loop only advances the watermark when the batch wrote successfully. Failed batches log a debug message and continue so the loop still gets through the rest. backfillAllProjects() now constructs SessionStore and ChromaSync inside a try block so a constructor throw can't leave the static backfillInProgress guard stuck true and silently skip every later backfill. The finally always clears the guard and best-effort closes each resource. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: fall back to pid kill when process group is gone Fixes CodeRabbit outside-diff comment on src/supervisor/shutdown.ts:118-134 from PR #2282 review. signalProcess() returned silently when a pgid was present and process.kill(-pgid, signal) threw ESRCH, never attempting the per-pid signal. With the new chroma registration path that records a pgid alongside the pid, an already-collapsed group could turn shutdown into a no-op even though the root pid was still alive. The POSIX branch now tries -pgid first when present, and on ESRCH falls through to process.kill(pid, signal). Non-ESRCH errors still propagate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: settings path, uptime clamp, fetch timeouts Fixes three smaller CodeRabbit issues on PR #2282: - SettingsRoutes (outside-diff #2282 review on lines 65-79): the parse-error response told users to delete ~/.claude-mem/settings.json even when paths.settings() resolved elsewhere. Now uses the resolved settingsPath variable in the message. - uptime.ts (#3178957264 / lines 2-3): getUptimeSeconds() could return a negative value if startedAtMs was in the future or the system clock moved backward. Clamps with Math.max(0, ...) so health endpoints never see negative seconds. - check-pending-queue.ts (#3178957248 / lines 27-45): checkWorkerHealth, getProcessingStatus and triggerProcessing all called fetch with no timeout, so the script could block forever if the worker accepted the TCP connection but never responded. Wraps each fetch with an AbortController + 10s timeout that throws a clear timeout message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: walk descendants recursively when killing chroma-mcp tree Fixes CodeRabbit comment #3178957258 on PR #2282. The POSIX teardown in ChromaMcpManager.killProcessTree() relied on `pkill -P <pid>`, which only signals direct children. Under uv, chroma-mcp spawns python as a grandchild — when uv exits and python re-parents to init, pkill -P never reaches it and the descendant survives the "tree kill". killProcessTree() now collects the full descendant set via a recursive `pgrep -P` walk before each signal phase. The walk returns leaves first so signals propagate bottom-up (SIGTERM children before their parents, then again for SIGKILL after the 500ms grace window so any layer that re-parented during teardown still gets cleaned up). pgrep failures (no children, missing binary) return [] so this stays best-effort and falls back to the existing per-pid signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: tolerate malformed JSONL lines in transcript-parser Fixes Greptile P1 comment 3178964456 on PR #2282. extractLastMessageFromJsonl previously called JSON.parse(rawLine) with no guard. A truncated/malformed JSONL line — common when a transcript was crashed mid-write or partially flushed — would throw SyntaxError, crash the summarization pipeline for that session, and silently lose all prior valid messages. Fix: wrap JSON.parse in try/catch and skip bad lines. The empty-line guard only catches truly empty strings, not malformed fragments. Regression tests added for two cases: - Mixed valid + truncated lines: returns last valid match. - All lines malformed: returns empty string (no throw). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: classify FK constraint failures BEFORE provider classifier Fixes Greptile P1 comment 3178979583 on PR #2282. The F4 #2244 work introduced a regression: reclassifyAtDispatch always returns a non-null ClassifiedProviderError for known agent types (Claude/Gemini/OpenRouter), so the isFkConstraintFailure branch was dead code. Per-provider classifiers don't recognize "FOREIGN KEY constraint failed", so SQLite FK failures fell through to the default 'transient' kind and would retry indefinitely — restart loop on corrupted session DB state. Old unrecoverablePatterns explicitly listed FK constraint as unrecoverable; restoring that semantic by checking FK FIRST and only deferring to the classifier when not an FK error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: validate CLAUDE_MEM_WORKER_PORT in check-pending-queue Parse the env var, range-check (1-65535), and fall back to 37777 with a console.warn on invalid input instead of letting a malformed value flow into the URL builder unchecked (CodeRabbit Minor on PR #2282). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: SIGKILL union of pre-TERM and post-wait descendant sets When the chroma-mcp root exits during the SIGTERM grace window, its descendants get re-parented to init and drop out of the post-wait pgrep -P scan. Without including the pre-TERM snapshot, those re-parented PIDs would never receive SIGKILL even though they were definitely children before SIGTERM and may still be alive (CodeRabbit Major on PR #2282). Compute Array.from(new Set([...descendantsBeforeTerm, ...descendantsBeforeKill])) and SIGKILL the union. The two sets typically overlap, so dedupe is required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: enforce addDocuments return-count in direct sync paths syncObservation/syncSummary/syncUserPrompt now capture the written count from addDocuments() and only bump the watermark when every requested document landed in Chroma. addDocuments() tolerates per-batch failures (returns the actual written count), so the previous unconditional bump was silently marking unsynced rows as synced on transient errors — preventing the next backfill from retrying them (CodeRabbit Major on PR #2282). A partial write now logs a warn with the (requested, written) pair and preserves retryability on the next pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: guard backfill watermark against non-contiguous failures The backfill watermark is a single monotonic id, so it cannot represent sparse success: "synced through 200, gap at 201–250, then 251 onward" would, on restart, skip 201–250 forever because the watermark sat at either 200 or 251 — both lose data (CodeRabbit Major on PR #2282). Add a per-loop hadGap flag to backfillObservations / backfillSummaries / backfillPrompts. Once any batch under-writes, every subsequent batch must also skip the bump, regardless of whether it itself succeeded. Also tighten the failure check from `writtenInBatch <= 0` to `writtenInBatch < batch.length` so partial-batch writes are caught. The watermark stays at the last contiguously-synced position; the next backfill pass retries from there, eventually closing the gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: clear oauth-stale marker when token is absent When an OAuth token disappears entirely (user logs out, keychain cleared), buildIsolatedEnvWithFreshOAuth's absent branch was leaving any prior stale-marker file in place. The session-start hook would then keep surfacing an "expired token, re-login" warning even though the token is no longer expired — it's gone, and re-login was already done elsewhere or not applicable (CodeRabbit Minor on PR #2282). Call clearStaleMarker() in the absent branch the same way the present branch already does. Add a regression test exercising the full buildIsolatedEnvWithFreshOAuth path: pre-write a marker, force absent via spoofed unsupported platform, assert the marker is gone after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: skip unknown message.content shapes instead of throwing extractLastMessageFromJsonl already tolerates malformed JSONL lines (JSON.parse failure -> continue), but a valid JSON line whose message.content is an unexpected type (null, number, plain object) was still throwing — contradicting the new tolerance and crashing the entire summary pipeline on a single weird line (CodeRabbit Major + Greptile P1 on PR #2282). Replace the `throw new Error(...)` with `continue` so a single bad content shape skips that line instead of failing the whole transcript read. Forward compat: future content schemas land harmlessly. Add regression tests covering null, number, and plain-object content; each must not throw and must fall back to the most recent valid line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: guard null/primitive entries in message.content array Fixes CodeRabbit comment 3179004190 on PR #2282. The Array.isArray branch previously did `c.type === 'text'` directly, which throws if `c` is null or a primitive — possible in malformed logs. Tightened the filter with a type guard: requires c to be a non-null object with type === 'text' and a string text field. Same defensive class as the malformed-line and unknown-content-shape tolerances. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
Vendored
+5
@@ -0,0 +1,5 @@
|
||||
{"role":"user","message":{"content":[{"type":"text","text":"please list the files in src/"}]}}
|
||||
{"role":"assistant","message":{"content":[{"type":"text","text":"I'll list the files now."}]}}
|
||||
{"role":"user","message":{"content":[{"type":"text","text":"thanks, also tell me what you found"}]}}
|
||||
{"role":"assistant","message":{"content":[{"type":"tool_use","name":"Shell","input":{"command":"ls src/"}}]}}
|
||||
{"role":"assistant","message":{"content":[{"type":"text","text":"Here are the files: adapters, handlers, types."}]}}
|
||||
@@ -275,7 +275,11 @@ describe('GeminiProvider', () => {
|
||||
|
||||
global.fetch = mock(() => Promise.resolve(new Response('Invalid argument', { status: 400 })));
|
||||
|
||||
await expect(agent.startSession(session)).rejects.toThrow('Gemini API error: 400 - Invalid argument');
|
||||
// F4 classifyGeminiError surfaces 400 as a classified `unrecoverable` error
|
||||
// with a stable message ("Gemini bad request (status 400)") rather than
|
||||
// forwarding the raw upstream body. The original cause is preserved on
|
||||
// `.cause` for diagnostics — see classifyGeminiError in GeminiProvider.ts.
|
||||
await expect(agent.startSession(session)).rejects.toThrow('Gemini bad request (status 400)');
|
||||
});
|
||||
|
||||
it('should respect rate limits when rate limiting enabled', async () => {
|
||||
|
||||
@@ -51,6 +51,16 @@ describe('GracefulShutdown', () => {
|
||||
});
|
||||
|
||||
describe('performGracefulShutdown', () => {
|
||||
// Timeout bumped to 15s. performGracefulShutdown calls
|
||||
// getSupervisor().stop() which runs runShutdownCascade against the real
|
||||
// ~/.claude-mem/supervisor.json registry. If the developer has a live
|
||||
// worker + chroma-mcp registered, the cascade SIGTERMs/SIGKILLs them
|
||||
// and waits up to ~5–6s for them to exit, which sails past the default
|
||||
// 5000ms test timeout. The other shutdown tests below are unaffected
|
||||
// because they don't register an mcpClient/dbManager/chromaMcpManager
|
||||
// mock that exercises the same path. This is test-infrastructure debt
|
||||
// — the test interacts with the production supervisor singleton — not
|
||||
// a code regression in the shutdown flow itself.
|
||||
it('should call shutdown steps in correct order', async () => {
|
||||
const callOrder: string[] = [];
|
||||
|
||||
@@ -115,7 +125,7 @@ describe('GracefulShutdown', () => {
|
||||
expect(callOrder.indexOf('mcpClient.close')).toBeLessThan(callOrder.indexOf('dbManager.close'));
|
||||
|
||||
expect(callOrder.indexOf('chromaMcpManager.stop')).toBeLessThan(callOrder.indexOf('dbManager.close'));
|
||||
});
|
||||
}, 15000);
|
||||
|
||||
it('should remove PID file during shutdown', async () => {
|
||||
const mockSessionManager: ShutdownableService = {
|
||||
|
||||
@@ -154,3 +154,82 @@ describe('parseAgentXml — observations', () => {
|
||||
expect(result[0].files_modified).toEqual(['src/utils.ts']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseAgentXml — fence tolerance (#2233 Part A)', () => {
|
||||
it('parses plain XML input correctly (no fence)', () => {
|
||||
const xml = `<observation>
|
||||
<type>discovery</type>
|
||||
<title>Plain XML input</title>
|
||||
<narrative>No fence wrapper present.</narrative>
|
||||
</observation>`;
|
||||
|
||||
const result = parseAgentXml(xml);
|
||||
expect(result.valid).toBe(true);
|
||||
if (!result.valid) return;
|
||||
expect(result.observations).toHaveLength(1);
|
||||
expect(result.observations[0].title).toBe('Plain XML input');
|
||||
});
|
||||
|
||||
it('parses fenced XML with language tag (```xml ... ```)', () => {
|
||||
const xml = '```xml\n<observation>\n <type>discovery</type>\n <title>Fenced with lang</title>\n <narrative>Wrapped in xml-tagged code fence.</narrative>\n</observation>\n```';
|
||||
|
||||
const result = parseAgentXml(xml);
|
||||
expect(result.valid).toBe(true);
|
||||
if (!result.valid) return;
|
||||
expect(result.observations).toHaveLength(1);
|
||||
expect(result.observations[0].title).toBe('Fenced with lang');
|
||||
expect(result.observations[0].narrative).toBe('Wrapped in xml-tagged code fence.');
|
||||
});
|
||||
|
||||
it('parses fenced XML without language tag (``` ... ```)', () => {
|
||||
const xml = '```\n<observation>\n <type>bugfix</type>\n <title>Bare fence</title>\n <narrative>Wrapped in language-less fence.</narrative>\n</observation>\n```';
|
||||
|
||||
const result = parseAgentXml(xml);
|
||||
expect(result.valid).toBe(true);
|
||||
if (!result.valid) return;
|
||||
expect(result.observations).toHaveLength(1);
|
||||
expect(result.observations[0].title).toBe('Bare fence');
|
||||
expect(result.observations[0].narrative).toBe('Wrapped in language-less fence.');
|
||||
});
|
||||
|
||||
it('does not falsely strip when XML appears mid-text without fences', () => {
|
||||
const xml = `Some intro prose.
|
||||
<observation>
|
||||
<type>refactor</type>
|
||||
<title>Mid-text observation</title>
|
||||
<narrative>No fences anywhere in the input.</narrative>
|
||||
</observation>
|
||||
Trailing prose.`;
|
||||
|
||||
const result = parseAgentXml(xml);
|
||||
expect(result.valid).toBe(true);
|
||||
if (!result.valid) return;
|
||||
expect(result.observations).toHaveLength(1);
|
||||
expect(result.observations[0].title).toBe('Mid-text observation');
|
||||
expect(result.observations[0].narrative).toBe('No fences anywhere in the input.');
|
||||
});
|
||||
|
||||
it('does not strip inner triple-backtick lines when payload is not a full fenced wrapper', () => {
|
||||
// Regression for CodeRabbit review on PR #2282: stripCodeFences() used to
|
||||
// greedily remove the first ``` and last ``` anywhere in the input, which
|
||||
// could mangle content that contains internal fenced examples or surrounds
|
||||
// the XML with prose. The fence-stripper must only fire when the entire
|
||||
// payload is a single fenced block.
|
||||
const xml = 'Lead-in text with ```inline``` markers.\n' +
|
||||
'<observation>\n' +
|
||||
' <type>discovery</type>\n' +
|
||||
' <title>Body with ``` inside narrative</title>\n' +
|
||||
' <narrative>Snippet: ```\nfoo\n``` end of snippet.</narrative>\n' +
|
||||
'</observation>\n' +
|
||||
'Trailing ``` prose with another ``` mark.';
|
||||
|
||||
const result = parseAgentXml(xml);
|
||||
expect(result.valid).toBe(true);
|
||||
if (!result.valid) return;
|
||||
expect(result.observations).toHaveLength(1);
|
||||
expect(result.observations[0].title).toBe('Body with ``` inside narrative');
|
||||
// Narrative should still contain the inner ``` markers — i.e. the
|
||||
// stripper did not eat them.
|
||||
expect(result.observations[0].narrative).toContain('```');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,288 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test';
|
||||
import * as childProcess from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import { join } from 'path';
|
||||
import {
|
||||
readClaudeOAuthToken,
|
||||
decodeJwtExpMs,
|
||||
writeStaleMarker,
|
||||
clearStaleMarker,
|
||||
readStaleMarker,
|
||||
} from '../../src/shared/oauth-token.js';
|
||||
import { paths } from '../../src/shared/paths.js';
|
||||
import { buildIsolatedEnvWithFreshOAuth } from '../../src/shared/EnvManager.js';
|
||||
|
||||
/**
|
||||
* The implementation uses promisify(execFile), which captures execFile at
|
||||
* module-load time. To intercept those calls in tests we replace the export
|
||||
* on `child_process` and restore it afterwards. We also redirect DATA_DIR
|
||||
* to a per-test temp dir for marker/sidecar tests.
|
||||
*/
|
||||
|
||||
const ORIGINAL_EXEC_FILE = childProcess.execFile;
|
||||
const ORIGINAL_PLATFORM = process.platform;
|
||||
const ORIGINAL_ENV_TOKEN = process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
const ORIGINAL_DATA_DIR = process.env.CLAUDE_MEM_DATA_DIR;
|
||||
|
||||
let dataDirSpy: ReturnType<typeof spyOn> | undefined;
|
||||
let tempDir: string;
|
||||
|
||||
function setPlatform(value: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, 'platform', { value, configurable: true });
|
||||
}
|
||||
|
||||
function restorePlatform(): void {
|
||||
Object.defineProperty(process, 'platform', { value: ORIGINAL_PLATFORM, configurable: true });
|
||||
}
|
||||
|
||||
/**
|
||||
* Patch promisify(execFile) by replacing the underlying execFile with a stub
|
||||
* that calls back like the real Node API. Because oauth-token.ts already
|
||||
* captured the original at import time, we instead intercept the cached
|
||||
* promisified function via the module's internal binding by re-importing.
|
||||
*
|
||||
* Simpler approach: spy on childProcess.execFile and route calls to a fake
|
||||
* callback. Because promisify wraps execFile by reference at import time,
|
||||
* we can't intercept post-hoc. Instead we exercise the parsing logic
|
||||
* directly via parseKeychainPayload-equivalent paths: we inject results by
|
||||
* calling readClaudeOAuthToken() with platform spoofed AND the expected
|
||||
* `security`/`secret-tool` binary spy via mocking the `execFile` hostpath.
|
||||
*
|
||||
* Bun's spyOn lets us replace properties on the `child_process` module
|
||||
* object, but the promisified handle inside oauth-token.ts already holds a
|
||||
* reference. So we test the parsing layer by exercising decodeJwtExpMs
|
||||
* directly and rely on environment-fallback path for the integration shape.
|
||||
*/
|
||||
|
||||
beforeEach(() => {
|
||||
// Redirect DATA_DIR to a temp directory for marker file tests.
|
||||
tempDir = fs.mkdtempSync(join(fs.realpathSync(require('os').tmpdir()), 'claude-mem-oauth-test-'));
|
||||
dataDirSpy = spyOn(paths, 'dataDir').mockImplementation(() => tempDir);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
dataDirSpy?.mockRestore();
|
||||
restorePlatform();
|
||||
if (ORIGINAL_ENV_TOKEN === undefined) {
|
||||
delete process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
} else {
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = ORIGINAL_ENV_TOKEN;
|
||||
}
|
||||
if (ORIGINAL_DATA_DIR === undefined) {
|
||||
delete process.env.CLAUDE_MEM_DATA_DIR;
|
||||
} else {
|
||||
process.env.CLAUDE_MEM_DATA_DIR = ORIGINAL_DATA_DIR;
|
||||
}
|
||||
// Clean up temp dir
|
||||
try {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
// best effort
|
||||
}
|
||||
});
|
||||
|
||||
describe('decodeJwtExpMs', () => {
|
||||
it('returns undefined for non-JWT tokens', () => {
|
||||
expect(decodeJwtExpMs('sk-ant-oat01-bare-token')).toBeUndefined();
|
||||
expect(decodeJwtExpMs('not.a.jwt.really')).toBeUndefined();
|
||||
expect(decodeJwtExpMs('')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('extracts exp claim from a real JWT and converts seconds to ms', () => {
|
||||
// header.payload.signature where payload is {"exp": 9999999999}
|
||||
const header = Buffer.from(JSON.stringify({ alg: 'none' })).toString('base64url');
|
||||
const payload = Buffer.from(JSON.stringify({ exp: 9999999999 })).toString('base64url');
|
||||
const signature = 'sig';
|
||||
const jwt = `${header}.${payload}.${signature}`;
|
||||
expect(decodeJwtExpMs(jwt)).toBe(9999999999 * 1000);
|
||||
});
|
||||
|
||||
it('returns undefined when JWT payload has no exp claim', () => {
|
||||
const header = Buffer.from(JSON.stringify({ alg: 'none' })).toString('base64url');
|
||||
const payload = Buffer.from(JSON.stringify({ sub: 'user' })).toString('base64url');
|
||||
const jwt = `${header}.${payload}.sig`;
|
||||
expect(decodeJwtExpMs(jwt)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined for malformed JWT', () => {
|
||||
expect(decodeJwtExpMs('not-base64.not-base64.sig')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('marker file scheme', () => {
|
||||
it('writeStaleMarker creates the marker file with the reason', () => {
|
||||
writeStaleMarker('token expired at 2026-01-01');
|
||||
const markerPath = join(tempDir, 'oauth-stale.marker');
|
||||
expect(fs.existsSync(markerPath)).toBe(true);
|
||||
expect(fs.readFileSync(markerPath, 'utf-8')).toBe('token expired at 2026-01-01');
|
||||
});
|
||||
|
||||
it('readStaleMarker returns undefined when no marker exists', () => {
|
||||
expect(readStaleMarker()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('readStaleMarker returns the reason after writeStaleMarker', () => {
|
||||
writeStaleMarker('refresh me');
|
||||
expect(readStaleMarker()).toBe('refresh me');
|
||||
});
|
||||
|
||||
it('clearStaleMarker removes an existing marker', () => {
|
||||
writeStaleMarker('temporary');
|
||||
expect(readStaleMarker()).toBe('temporary');
|
||||
clearStaleMarker();
|
||||
expect(readStaleMarker()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('clearStaleMarker is a no-op when no marker exists', () => {
|
||||
expect(() => clearStaleMarker()).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('readClaudeOAuthToken — env-fallback branch', () => {
|
||||
// These tests exercise the env-fallback path which is reachable on every
|
||||
// platform when the keychain returns absent. We force absent by spoofing
|
||||
// the platform to an unsupported value.
|
||||
beforeEach(() => {
|
||||
setPlatform('aix' as NodeJS.Platform); // unsupported -> always absent
|
||||
});
|
||||
|
||||
it('returns absent when no env token is set', async () => {
|
||||
delete process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(result.kind).toBe('absent');
|
||||
if (result.kind === 'absent') {
|
||||
expect(result.reason).toContain('Unsupported platform');
|
||||
}
|
||||
});
|
||||
|
||||
it('returns present (env-fallback) when env token is set and not expired', async () => {
|
||||
// Non-JWT bare token, no sidecar -> no expiresAt detectable -> not expired.
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat01-fallback';
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(result.kind).toBe('present');
|
||||
if (result.kind === 'present') {
|
||||
expect(result.token).toBe('sk-ant-oat01-fallback');
|
||||
expect(result.source).toBe('env-fallback');
|
||||
}
|
||||
});
|
||||
|
||||
it('returns expired when env token JWT exp claim is in the past', async () => {
|
||||
// Build a JWT with exp=1 (1970) — definitely expired.
|
||||
const header = Buffer.from(JSON.stringify({ alg: 'none' })).toString('base64url');
|
||||
const payload = Buffer.from(JSON.stringify({ exp: 1 })).toString('base64url');
|
||||
const expiredJwt = `${header}.${payload}.sig`;
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = expiredJwt;
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(result.kind).toBe('expired');
|
||||
if (result.kind === 'expired') {
|
||||
expect(result.reason).toContain('expired');
|
||||
expect(result.expiresAt).toBe(1000); // 1 sec * 1000
|
||||
}
|
||||
});
|
||||
|
||||
it('returns expired when sidecar metadata indicates env token is stale', async () => {
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat01-bare';
|
||||
// Write a sidecar with expiresAt in the past (well beyond grace window).
|
||||
const sidecarPath = join(tempDir, 'oauth-token-meta.json');
|
||||
const stalePastMs = Date.now() - 60 * 60 * 1000; // 1 hour ago
|
||||
fs.writeFileSync(sidecarPath, JSON.stringify({ expiresAt: stalePastMs }));
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(result.kind).toBe('expired');
|
||||
if (result.kind === 'expired') {
|
||||
expect(result.expiresAt).toBe(stalePastMs);
|
||||
}
|
||||
});
|
||||
|
||||
it('returns present when sidecar expiresAt is in the future', async () => {
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat01-bare';
|
||||
const sidecarPath = join(tempDir, 'oauth-token-meta.json');
|
||||
const futureMs = Date.now() + 60 * 60 * 1000; // 1 hour from now
|
||||
fs.writeFileSync(sidecarPath, JSON.stringify({ expiresAt: futureMs }));
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(result.kind).toBe('present');
|
||||
if (result.kind === 'present') {
|
||||
expect(result.expiresAt).toBe(futureMs);
|
||||
expect(result.source).toBe('env-fallback');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('readClaudeOAuthToken — macOS keychain branch', () => {
|
||||
// We can't easily intercept the cached promisified execFile from inside
|
||||
// oauth-token.ts (it captured a reference at module load). Instead we
|
||||
// verify the macOS branch dispatches by checking that on darwin without
|
||||
// a real keychain entry, the fallback path is reached.
|
||||
it('on macOS, falls back to env when keychain access fails or returns nothing', async () => {
|
||||
if (process.platform !== 'darwin') {
|
||||
// Skip on non-macOS — we only run this test where security CLI exists.
|
||||
return;
|
||||
}
|
||||
// Use an env token; if the real keychain has a fresh entry, we get
|
||||
// 'present' with source='keychain'. If no keychain entry, we fall back
|
||||
// to env-fallback. Either way, kind='present' with a non-empty token
|
||||
// (or 'expired' if the real keychain entry happens to be stale).
|
||||
process.env.CLAUDE_CODE_OAUTH_TOKEN = 'sk-ant-oat01-test-fallback';
|
||||
setPlatform('darwin');
|
||||
const result = await readClaudeOAuthToken();
|
||||
// Whatever the keychain says, the result should be a valid kind.
|
||||
expect(['present', 'expired', 'absent']).toContain(result.kind);
|
||||
if (result.kind === 'present') {
|
||||
expect(result.token.length).toBeGreaterThan(0);
|
||||
expect(['keychain', 'env-fallback']).toContain(result.source);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('readClaudeOAuthToken — Linux branch', () => {
|
||||
it('on linux without secret-tool, returns absent gracefully', async () => {
|
||||
if (process.platform !== 'linux') return; // skip on non-linux
|
||||
setPlatform('linux');
|
||||
delete process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
const result = await readClaudeOAuthToken();
|
||||
// If secret-tool is not installed or has no entry, returns absent.
|
||||
// If somehow present, we accept that too.
|
||||
expect(['present', 'expired', 'absent']).toContain(result.kind);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readClaudeOAuthToken — Windows branch', () => {
|
||||
it('on win32 without keychain entry, returns absent or env-fallback', async () => {
|
||||
if (process.platform !== 'win32') return; // skip on non-windows
|
||||
setPlatform('win32');
|
||||
delete process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
const result = await readClaudeOAuthToken();
|
||||
expect(['present', 'expired', 'absent']).toContain(result.kind);
|
||||
});
|
||||
});
|
||||
|
||||
// CodeRabbit Minor (PR #2282 follow-up): when the OAuth token is absent, any
|
||||
// previously-written stale marker is no longer accurate (the token is gone,
|
||||
// not expired). buildIsolatedEnvWithFreshOAuth must clear it on the absent
|
||||
// branch the same way it does on present.
|
||||
describe('buildIsolatedEnvWithFreshOAuth — absent token clears stale marker', () => {
|
||||
beforeEach(() => {
|
||||
setPlatform('aix' as NodeJS.Platform); // unsupported -> always absent
|
||||
delete process.env.CLAUDE_CODE_OAUTH_TOKEN;
|
||||
});
|
||||
|
||||
it('clears a pre-existing stale marker when token is absent', async () => {
|
||||
// Pre-existing marker from an earlier "expired" pass.
|
||||
writeStaleMarker('left over from previous run');
|
||||
expect(readStaleMarker()).toBe('left over from previous run');
|
||||
|
||||
// Force the absent path: ANTHROPIC_API_KEY must NOT be set in either the
|
||||
// env file or the process env, otherwise the early-return branch fires
|
||||
// before we ever reach the OAuth resolution.
|
||||
const origAnthropicKey = process.env.ANTHROPIC_API_KEY;
|
||||
delete process.env.ANTHROPIC_API_KEY;
|
||||
try {
|
||||
await buildIsolatedEnvWithFreshOAuth(true);
|
||||
} finally {
|
||||
if (origAnthropicKey !== undefined) {
|
||||
process.env.ANTHROPIC_API_KEY = origAnthropicKey;
|
||||
}
|
||||
}
|
||||
|
||||
expect(readStaleMarker()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import { paths, DATA_DIR } from '../../src/shared/paths.js';
|
||||
|
||||
describe('paths namespace', () => {
|
||||
it('exposes at least the known core accessors', () => {
|
||||
const keys = Object.keys(paths);
|
||||
const required = [
|
||||
'dataDir',
|
||||
'workerPid',
|
||||
'settings',
|
||||
'database',
|
||||
'chroma',
|
||||
'transcriptsConfig',
|
||||
];
|
||||
for (const key of required) {
|
||||
expect(keys).toContain(key);
|
||||
}
|
||||
});
|
||||
|
||||
it('every accessor returns a string starting with DATA_DIR', () => {
|
||||
for (const key of Object.keys(paths) as Array<keyof typeof paths>) {
|
||||
const value = paths[key]();
|
||||
expect(typeof value).toBe('string');
|
||||
expect(value.startsWith(DATA_DIR)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('every accessor is a callable function', () => {
|
||||
for (const key of Object.keys(paths) as Array<keyof typeof paths>) {
|
||||
expect(typeof paths[key]).toBe('function');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,225 @@
|
||||
/**
|
||||
* Regression tests for issue #2248: Cursor IDE sessions are never summarized.
|
||||
*
|
||||
* Validates the three fixes that make Cursor sessions actually get summarized
|
||||
* end-to-end (previously they were silently skipped):
|
||||
* A. cursor adapter derives `transcriptPath` from `cwd + conversation_id`,
|
||||
* since Cursor does not pass a transcript path on stdin.
|
||||
* B. `extractLastMessageFromJsonl` accepts both `{type:"assistant"}` (Claude
|
||||
* Code) and `{role:"assistant"}` (Cursor) per-line role markers.
|
||||
* C. `extractLastMessageFromJsonl` keeps scanning back through assistant
|
||||
* turns when the most recent one is a pure tool_use (no text content),
|
||||
* instead of returning an empty string and causing the summary to be
|
||||
* skipped.
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { readFileSync, writeFileSync, mkdirSync, rmSync, existsSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir, homedir } from 'os';
|
||||
import { extractLastMessage, extractLastMessageFromJsonl } from '../../src/shared/transcript-parser.js';
|
||||
import { cursorAdapter, deriveCursorTranscriptPath } from '../../src/cli/adapters/cursor.js';
|
||||
|
||||
const FIXTURE_PATH = join(__dirname, '..', 'fixtures', 'cursor-session.jsonl');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Bug B + C: extractLastMessageFromJsonl on the cursor-session.jsonl fixture
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('cursor-extraction: extractLastMessageFromJsonl on fixture', () => {
|
||||
const fixtureContent = readFileSync(FIXTURE_PATH, 'utf-8').trim();
|
||||
|
||||
it('returns the last user text from the fixture', () => {
|
||||
expect(extractLastMessageFromJsonl(fixtureContent, 'user', false)).toBe(
|
||||
'thanks, also tell me what you found'
|
||||
);
|
||||
});
|
||||
|
||||
it('returns the final assistant text (skipping tool_use-only turn)', () => {
|
||||
expect(extractLastMessageFromJsonl(fixtureContent, 'assistant', false)).toBe(
|
||||
'Here are the files: adapters, handlers, types.'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Bug B + C: extractLastMessage with extra inline cases
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('cursor-extraction: extractLastMessage Cursor JSONL compatibility', () => {
|
||||
const tmpDir = join(tmpdir(), `cursor-extraction-test-${Date.now()}`);
|
||||
const transcriptPath = join(tmpDir, 'transcript.jsonl');
|
||||
|
||||
beforeEach(() => {
|
||||
mkdirSync(tmpDir, { recursive: true });
|
||||
});
|
||||
afterEach(() => {
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('reads Cursor JSONL using {"role":"assistant"} (Bug B regression)', () => {
|
||||
const lines = [
|
||||
{ role: 'user', message: { content: [{ type: 'text', text: 'hello' }] } },
|
||||
{ role: 'assistant', message: { content: [{ type: 'text', text: 'hi from cursor' }] } },
|
||||
];
|
||||
writeFileSync(transcriptPath, lines.map((l) => JSON.stringify(l)).join('\n'));
|
||||
|
||||
expect(extractLastMessage(transcriptPath, 'assistant')).toBe('hi from cursor');
|
||||
});
|
||||
|
||||
it('skips a tool-only last assistant turn and returns the previous text-bearing one (Bug C regression)', () => {
|
||||
const lines = [
|
||||
{ role: 'user', message: { content: [{ type: 'text', text: 'q1' }] } },
|
||||
{ role: 'assistant', message: { content: [{ type: 'text', text: 'real answer' }] } },
|
||||
{ role: 'user', message: { content: [{ type: 'text', text: 'q2' }] } },
|
||||
{ role: 'assistant', message: { content: [{ type: 'tool_use', name: 'Shell', input: { command: 'ls' } }] } },
|
||||
];
|
||||
writeFileSync(transcriptPath, lines.map((l) => JSON.stringify(l)).join('\n'));
|
||||
|
||||
expect(extractLastMessage(transcriptPath, 'assistant')).toBe('real answer');
|
||||
});
|
||||
|
||||
it('still returns "" when no assistant turn exists at all', () => {
|
||||
const lines = [{ role: 'user', message: { content: [{ type: 'text', text: 'lonely' }] } }];
|
||||
writeFileSync(transcriptPath, lines.map((l) => JSON.stringify(l)).join('\n'));
|
||||
|
||||
expect(extractLastMessage(transcriptPath, 'assistant')).toBe('');
|
||||
});
|
||||
|
||||
it('still works for Claude Code format using {"type":"assistant"}', () => {
|
||||
const lines = [
|
||||
{ type: 'user', message: { content: [{ type: 'text', text: 'q' }] } },
|
||||
{ type: 'assistant', message: { content: [{ type: 'text', text: 'claude code answer' }] } },
|
||||
];
|
||||
writeFileSync(transcriptPath, lines.map((l) => JSON.stringify(l)).join('\n'));
|
||||
|
||||
expect(extractLastMessage(transcriptPath, 'assistant')).toBe('claude code answer');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Bug A: cursor adapter transcript path derivation
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('cursor-extraction: cursorAdapter transcriptPath derivation', () => {
|
||||
const sessionId = `c0ffee${Date.now()}`;
|
||||
const fakeCwd = join(tmpdir(), 'fake.workspace', 'subdir');
|
||||
const slug = fakeCwd.replace(/^\//, '').replace(/[/.]/g, '-');
|
||||
const transcriptDir = join(homedir(), '.cursor', 'projects', slug, 'agent-transcripts', sessionId);
|
||||
const transcriptPath = join(transcriptDir, `${sessionId}.jsonl`);
|
||||
|
||||
beforeEach(() => {
|
||||
mkdirSync(fakeCwd, { recursive: true });
|
||||
mkdirSync(transcriptDir, { recursive: true });
|
||||
writeFileSync(
|
||||
transcriptPath,
|
||||
JSON.stringify({ role: 'assistant', message: { content: [{ type: 'text', text: 'ok' }] } }) + '\n'
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (existsSync(transcriptPath)) rmSync(transcriptPath);
|
||||
if (existsSync(transcriptDir)) rmSync(transcriptDir, { recursive: true, force: true });
|
||||
if (existsSync(fakeCwd)) rmSync(fakeCwd, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('derives transcriptPath from cwd + conversation_id when the file exists (Bug A regression)', () => {
|
||||
const normalized = cursorAdapter.normalizeInput({
|
||||
cwd: fakeCwd,
|
||||
conversation_id: sessionId,
|
||||
});
|
||||
|
||||
expect(normalized.sessionId).toBe(sessionId);
|
||||
expect(normalized.transcriptPath).toBe(transcriptPath);
|
||||
});
|
||||
|
||||
it('returns transcriptPath: undefined when the file does not exist', () => {
|
||||
rmSync(transcriptPath);
|
||||
const normalized = cursorAdapter.normalizeInput({
|
||||
cwd: fakeCwd,
|
||||
conversation_id: sessionId,
|
||||
});
|
||||
|
||||
expect(normalized.sessionId).toBe(sessionId);
|
||||
expect(normalized.transcriptPath).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined when sessionId is missing (deriveCursorTranscriptPath direct call)', () => {
|
||||
expect(deriveCursorTranscriptPath(fakeCwd, undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined when cwd is missing (deriveCursorTranscriptPath direct call)', () => {
|
||||
expect(deriveCursorTranscriptPath(undefined, sessionId)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Greptile P1 (PR #2282): malformed JSONL lines must not crash the pipeline
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('cursor-extraction: malformed JSONL tolerance', () => {
|
||||
it('skips truncated/malformed lines and returns the last valid match', () => {
|
||||
const validLine = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: [{ type: 'text', text: 'recovered text' }] },
|
||||
});
|
||||
const malformed = '{"role":"assistant","message":{"content":[{"type":"tex'; // truncated mid-write
|
||||
const content = [validLine, malformed].join('\n');
|
||||
|
||||
expect(() => extractLastMessageFromJsonl(content, 'assistant', false)).not.toThrow();
|
||||
expect(extractLastMessageFromJsonl(content, 'assistant', false)).toBe('recovered text');
|
||||
});
|
||||
|
||||
it('returns empty string when ALL lines are malformed', () => {
|
||||
const content = ['{partial', 'not even close to json', '}{'].join('\n');
|
||||
expect(extractLastMessageFromJsonl(content, 'assistant', false)).toBe('');
|
||||
});
|
||||
|
||||
// CodeRabbit Major + Greptile P1 (PR #2282 follow-up): a valid JSON line
|
||||
// whose `message.content` is an unexpected type (null, number, plain
|
||||
// object) used to throw. It must now be skipped — same tolerance class as
|
||||
// truncated lines.
|
||||
it('skips a line whose message.content is null and falls back to a valid earlier line', () => {
|
||||
const valid = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: [{ type: 'text', text: 'kept' }] },
|
||||
});
|
||||
const nullContent = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: null },
|
||||
});
|
||||
const content = [valid, nullContent].join('\n');
|
||||
|
||||
expect(() => extractLastMessageFromJsonl(content, 'assistant', false)).not.toThrow();
|
||||
expect(extractLastMessageFromJsonl(content, 'assistant', false)).toBe('kept');
|
||||
});
|
||||
|
||||
it('skips a line whose message.content is a number without throwing', () => {
|
||||
const valid = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: [{ type: 'text', text: 'kept too' }] },
|
||||
});
|
||||
const numericContent = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: 42 },
|
||||
});
|
||||
const content = [valid, numericContent].join('\n');
|
||||
|
||||
expect(() => extractLastMessageFromJsonl(content, 'assistant', false)).not.toThrow();
|
||||
expect(extractLastMessageFromJsonl(content, 'assistant', false)).toBe('kept too');
|
||||
});
|
||||
|
||||
it('skips a line whose message.content is a plain object without throwing', () => {
|
||||
const valid = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: [{ type: 'text', text: 'survivor' }] },
|
||||
});
|
||||
const objectContent = JSON.stringify({
|
||||
role: 'assistant',
|
||||
message: { content: { unexpected: 'shape' } },
|
||||
});
|
||||
const content = [valid, objectContent].join('\n');
|
||||
|
||||
expect(() => extractLastMessageFromJsonl(content, 'assistant', false)).not.toThrow();
|
||||
expect(extractLastMessageFromJsonl(content, 'assistant', false)).toBe('survivor');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,238 @@
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import {
|
||||
ClassifiedProviderError,
|
||||
isClassified,
|
||||
} from '../../src/services/worker/provider-errors.js';
|
||||
import { classifyClaudeError } from '../../src/services/worker/ClaudeProvider.js';
|
||||
import { classifyGeminiError } from '../../src/services/worker/GeminiProvider.js';
|
||||
import { classifyOpenRouterError } from '../../src/services/worker/OpenRouterProvider.js';
|
||||
|
||||
// Hard cases per F4 spec — provider-specific classifiers must map raw HTTP
|
||||
// shapes / SDK errors to ClassifiedProviderError with the right kind.
|
||||
|
||||
describe('classifyGeminiError', () => {
|
||||
it('classifies 429 with no Retry-After as rate_limit with no retryAfterMs', () => {
|
||||
const headers = new Headers(); // no Retry-After
|
||||
const cause = new Error('Gemini API error: 429 - quota');
|
||||
const err = classifyGeminiError({
|
||||
status: 429,
|
||||
bodyText: 'Too Many Requests',
|
||||
headers,
|
||||
cause,
|
||||
});
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
expect(err.cause).toBe(cause);
|
||||
});
|
||||
|
||||
it('classifies 429 with Retry-After: 5 as rate_limit with retryAfterMs=5000', () => {
|
||||
const headers = new Headers({ 'Retry-After': '5' });
|
||||
const err = classifyGeminiError({
|
||||
status: 429,
|
||||
bodyText: '',
|
||||
headers,
|
||||
cause: new Error('rate limited'),
|
||||
});
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
expect(err.retryAfterMs).toBe(5000);
|
||||
});
|
||||
|
||||
it('classifies 500 with body containing "quota exceeded" as quota_exhausted', () => {
|
||||
const err = classifyGeminiError({
|
||||
status: 500,
|
||||
bodyText: 'Internal: quota exceeded for model',
|
||||
cause: new Error('500 - quota exceeded'),
|
||||
});
|
||||
expect(err.kind).toBe('quota_exhausted');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('classifies 401 with "API key not valid" body as auth_invalid', () => {
|
||||
const err = classifyGeminiError({
|
||||
status: 401,
|
||||
bodyText: 'API key not valid. Please pass a valid API key.',
|
||||
cause: new Error('401'),
|
||||
});
|
||||
expect(err.kind).toBe('auth_invalid');
|
||||
});
|
||||
|
||||
it('classifies 403 PERMISSION_DENIED as auth_invalid', () => {
|
||||
const err = classifyGeminiError({
|
||||
status: 403,
|
||||
bodyText: 'PERMISSION_DENIED',
|
||||
cause: new Error('403'),
|
||||
});
|
||||
expect(err.kind).toBe('auth_invalid');
|
||||
});
|
||||
|
||||
it('classifies 503 as transient', () => {
|
||||
const err = classifyGeminiError({
|
||||
status: 503,
|
||||
bodyText: 'service unavailable',
|
||||
cause: new Error('503'),
|
||||
});
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies network error (no status) as transient', () => {
|
||||
const cause = new Error('fetch failed: ECONNREFUSED');
|
||||
const err = classifyGeminiError({ cause });
|
||||
expect(err.kind).toBe('transient');
|
||||
expect(err.cause).toBe(cause);
|
||||
});
|
||||
|
||||
it('classifies 400 as unrecoverable', () => {
|
||||
const err = classifyGeminiError({
|
||||
status: 400,
|
||||
bodyText: 'INVALID_ARGUMENT',
|
||||
cause: new Error('400'),
|
||||
});
|
||||
expect(err.kind).toBe('unrecoverable');
|
||||
});
|
||||
});
|
||||
|
||||
describe('classifyOpenRouterError', () => {
|
||||
it('classifies 429 with no Retry-After as rate_limit with no retryAfterMs', () => {
|
||||
const headers = new Headers(); // no Retry-After
|
||||
const err = classifyOpenRouterError({
|
||||
status: 429,
|
||||
bodyText: 'rate limit exceeded',
|
||||
headers,
|
||||
cause: new Error('429'),
|
||||
});
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('classifies 429 with Retry-After: 10 as rate_limit with retryAfterMs=10000', () => {
|
||||
const headers = new Headers({ 'retry-after': '10' });
|
||||
const err = classifyOpenRouterError({
|
||||
status: 429,
|
||||
bodyText: '',
|
||||
headers,
|
||||
cause: new Error('429'),
|
||||
});
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
expect(err.retryAfterMs).toBe(10_000);
|
||||
});
|
||||
|
||||
it('classifies 500 with body containing "quota exceeded" as quota_exhausted', () => {
|
||||
const err = classifyOpenRouterError({
|
||||
status: 500,
|
||||
bodyText: 'something quota exceeded',
|
||||
cause: new Error('500'),
|
||||
});
|
||||
expect(err.kind).toBe('quota_exhausted');
|
||||
});
|
||||
|
||||
it('classifies "insufficient credits" body as quota_exhausted regardless of status', () => {
|
||||
const err = classifyOpenRouterError({
|
||||
status: 402,
|
||||
bodyText: 'insufficient credits',
|
||||
cause: new Error('402'),
|
||||
});
|
||||
expect(err.kind).toBe('quota_exhausted');
|
||||
});
|
||||
|
||||
it('classifies 401 as auth_invalid', () => {
|
||||
const err = classifyOpenRouterError({
|
||||
status: 401,
|
||||
bodyText: 'unauthorized',
|
||||
cause: new Error('401'),
|
||||
});
|
||||
expect(err.kind).toBe('auth_invalid');
|
||||
});
|
||||
|
||||
it('classifies 502 as transient', () => {
|
||||
const err = classifyOpenRouterError({
|
||||
status: 502,
|
||||
bodyText: 'bad gateway',
|
||||
cause: new Error('502'),
|
||||
});
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies network error (no status) as transient', () => {
|
||||
const cause = new Error('ECONNRESET');
|
||||
const err = classifyOpenRouterError({ cause });
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
});
|
||||
|
||||
describe('classifyClaudeError', () => {
|
||||
it('classifies SDK-level OverloadedError as transient', () => {
|
||||
class OverloadedError extends Error {
|
||||
constructor() {
|
||||
super('Overloaded');
|
||||
this.name = 'OverloadedError';
|
||||
}
|
||||
}
|
||||
const err = classifyClaudeError(new OverloadedError());
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies 529 status as transient', () => {
|
||||
const sdkErr = Object.assign(new Error('overloaded'), { status: 529 });
|
||||
const err = classifyClaudeError(sdkErr);
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies anthropic error.type=overloaded_error as transient', () => {
|
||||
const sdkErr = Object.assign(new Error('upstream'), {
|
||||
error: { type: 'overloaded_error' },
|
||||
});
|
||||
const err = classifyClaudeError(sdkErr);
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies "Invalid API key" message as auth_invalid', () => {
|
||||
const err = classifyClaudeError(new Error('Invalid API key: configure ~/.claude-mem/.env'));
|
||||
expect(err.kind).toBe('auth_invalid');
|
||||
});
|
||||
|
||||
it('classifies status=401 as auth_invalid', () => {
|
||||
const sdkErr = Object.assign(new Error('unauthorized'), { status: 401 });
|
||||
const err = classifyClaudeError(sdkErr);
|
||||
expect(err.kind).toBe('auth_invalid');
|
||||
});
|
||||
|
||||
it('classifies ENOENT spawn error as unrecoverable', () => {
|
||||
const spawnErr = Object.assign(new Error('spawn claude ENOENT'), { code: 'ENOENT' });
|
||||
const err = classifyClaudeError(spawnErr);
|
||||
expect(err.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('classifies "Claude executable not found" as unrecoverable', () => {
|
||||
const err = classifyClaudeError(new Error('Claude executable not found at $CLAUDE_CODE_PATH'));
|
||||
expect(err.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('classifies prompt-too-long as unrecoverable', () => {
|
||||
const err = classifyClaudeError(new Error('Claude session context overflow: prompt is too long'));
|
||||
expect(err.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('classifies status=429 as rate_limit', () => {
|
||||
const sdkErr = Object.assign(new Error('rate limited'), { status: 429 });
|
||||
const err = classifyClaudeError(sdkErr);
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
});
|
||||
|
||||
it('classifies "quota exceeded" message as quota_exhausted', () => {
|
||||
const err = classifyClaudeError(new Error('upstream: quota exceeded'));
|
||||
expect(err.kind).toBe('quota_exhausted');
|
||||
});
|
||||
|
||||
it('classifies status=503 as transient', () => {
|
||||
const sdkErr = Object.assign(new Error('service unavailable'), { status: 503 });
|
||||
const err = classifyClaudeError(sdkErr);
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('classifies unknown error as transient (preserve old default)', () => {
|
||||
const err = classifyClaudeError(new Error('something weird happened'));
|
||||
expect(err.kind).toBe('transient');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,118 @@
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import {
|
||||
ClassifiedProviderError,
|
||||
isClassified,
|
||||
type ProviderErrorClass,
|
||||
} from '../../src/services/worker/provider-errors.js';
|
||||
|
||||
// These tests exercise the *type system* and *invariants of the class itself*.
|
||||
// Per-provider classification helpers (the actual mapping from raw SDK errors
|
||||
// to ClassifiedProviderError) come in a later task — here we feed stub inputs
|
||||
// representing what those helpers will eventually produce.
|
||||
|
||||
describe('ClassifiedProviderError', () => {
|
||||
it('classifies a 429-with-no-Retry-After response as rate_limit with no retryAfterMs', () => {
|
||||
const stubRaw = {
|
||||
status: 429,
|
||||
headers: {}, // no Retry-After header
|
||||
body: 'Too Many Requests',
|
||||
};
|
||||
|
||||
const err = new ClassifiedProviderError('rate limited', {
|
||||
kind: 'rate_limit',
|
||||
cause: stubRaw,
|
||||
});
|
||||
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('rate_limit');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
expect(err.cause).toBe(stubRaw);
|
||||
expect(err.message).toBe('rate limited');
|
||||
expect(err.name).toBe('ClassifiedProviderError');
|
||||
});
|
||||
|
||||
it('classifies a 500-with-quota-exceeded body as quota_exhausted', () => {
|
||||
const stubRaw = {
|
||||
status: 500,
|
||||
body: 'Internal error: quota exceeded for project',
|
||||
};
|
||||
|
||||
const err = new ClassifiedProviderError('quota exceeded', {
|
||||
kind: 'quota_exhausted',
|
||||
cause: stubRaw,
|
||||
});
|
||||
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('quota_exhausted');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
expect(err.cause).toBe(stubRaw);
|
||||
});
|
||||
|
||||
it('classifies an SDK-level OverloadedError as transient', () => {
|
||||
// Stand-in for an SDK error class instance (e.g. Anthropic OverloadedError).
|
||||
class OverloadedError extends Error {
|
||||
constructor() {
|
||||
super('Overloaded');
|
||||
this.name = 'OverloadedError';
|
||||
}
|
||||
}
|
||||
const stubRaw = new OverloadedError();
|
||||
|
||||
const err = new ClassifiedProviderError('upstream overloaded', {
|
||||
kind: 'transient',
|
||||
cause: stubRaw,
|
||||
retryAfterMs: 2000,
|
||||
});
|
||||
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('transient');
|
||||
expect(err.retryAfterMs).toBe(2000);
|
||||
expect(err.cause).toBe(stubRaw);
|
||||
});
|
||||
|
||||
it('classifies an unknown 4xx as unrecoverable', () => {
|
||||
const stubRaw = {
|
||||
status: 418,
|
||||
body: "I'm a teapot",
|
||||
};
|
||||
|
||||
const err = new ClassifiedProviderError('unrecoverable client error', {
|
||||
kind: 'unrecoverable',
|
||||
cause: stubRaw,
|
||||
});
|
||||
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('unrecoverable');
|
||||
expect(err.retryAfterMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('round-trips a custom string kind through the open-union type system', () => {
|
||||
// The (string & {}) branch in ProviderErrorClass means any string is
|
||||
// assignable, but the named literals still autocomplete. Verify that a
|
||||
// provider-specific kind survives unchanged through construction +
|
||||
// the isClassified guard, and that it satisfies the type.
|
||||
const customKind: ProviderErrorClass = 'flue_specific';
|
||||
|
||||
const err = new ClassifiedProviderError('flue-specific failure', {
|
||||
kind: customKind,
|
||||
cause: { provider: 'flue', code: 'F-42' },
|
||||
});
|
||||
|
||||
expect(isClassified(err)).toBe(true);
|
||||
expect(err.kind).toBe('flue_specific');
|
||||
|
||||
// Narrowing through isClassified preserves the kind field as ProviderErrorClass.
|
||||
if (isClassified(err)) {
|
||||
const k: ProviderErrorClass = err.kind;
|
||||
expect(k).toBe('flue_specific');
|
||||
}
|
||||
});
|
||||
|
||||
it('isClassified rejects non-ClassifiedProviderError values', () => {
|
||||
expect(isClassified(new Error('plain'))).toBe(false);
|
||||
expect(isClassified('rate_limit')).toBe(false);
|
||||
expect(isClassified(null)).toBe(false);
|
||||
expect(isClassified(undefined)).toBe(false);
|
||||
expect(isClassified({ kind: 'rate_limit' })).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,217 @@
|
||||
import { describe, it, expect, beforeEach } from 'bun:test';
|
||||
import {
|
||||
RateLimitStore,
|
||||
shouldAbortForQuota,
|
||||
isApiKeyAuth,
|
||||
type RateLimitInfo,
|
||||
} from '../../src/services/worker/RateLimitStore.js';
|
||||
|
||||
// Quota-aware wall-clock guard (#2234).
|
||||
//
|
||||
// Subscription users (cli/oauth) get aborted when they cross per-window
|
||||
// utilization thresholds, plus a reset-grace buffer for the rolling 5h
|
||||
// window. API-key users are exempt because they authorized per-call spend.
|
||||
|
||||
const FIXED_NOW = 1_700_000_000_000; // arbitrary epoch ms anchor
|
||||
|
||||
function freshStore(): RateLimitStore {
|
||||
return new RateLimitStore();
|
||||
}
|
||||
|
||||
describe('RateLimitStore', () => {
|
||||
it('records and retrieves entries by rateLimitType', () => {
|
||||
const store = freshStore();
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.5, status: 'allowed' });
|
||||
const got = store.get('five_hour');
|
||||
expect(got?.utilization).toBe(0.5);
|
||||
expect(got?.status).toBe('allowed');
|
||||
expect(typeof got?.observedAt).toBe('number');
|
||||
});
|
||||
|
||||
it('overwrites older entries for the same window (last-write-wins)', () => {
|
||||
const store = freshStore();
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.5 });
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.9 });
|
||||
expect(store.get('five_hour')?.utilization).toBe(0.9);
|
||||
});
|
||||
|
||||
it('keeps separate buckets per window', () => {
|
||||
const store = freshStore();
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.4 });
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.7 });
|
||||
expect(store.get('five_hour')?.utilization).toBe(0.4);
|
||||
expect(store.get('seven_day_opus')?.utilization).toBe(0.7);
|
||||
expect(store.size).toBe(2);
|
||||
});
|
||||
|
||||
it('falls back to "default" bucket when rateLimitType is missing', () => {
|
||||
const store = freshStore();
|
||||
store.set({ utilization: 0.6 } as RateLimitInfo);
|
||||
expect(store.get(undefined)?.utilization).toBe(0.6);
|
||||
});
|
||||
|
||||
it('ignores null/undefined input', () => {
|
||||
const store = freshStore();
|
||||
store.set(null as any);
|
||||
store.set(undefined as any);
|
||||
expect(store.size).toBe(0);
|
||||
});
|
||||
|
||||
it('getMostRecentByWindow returns latest snapshots keyed by window', () => {
|
||||
const store = freshStore();
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.1 });
|
||||
store.set({ rateLimitType: 'seven_day_sonnet', utilization: 0.2 });
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.3 });
|
||||
const snap = store.getMostRecentByWindow();
|
||||
expect(snap.five_hour?.utilization).toBe(0.1);
|
||||
expect(snap.seven_day_sonnet?.utilization).toBe(0.2);
|
||||
expect(snap.seven_day_opus?.utilization).toBe(0.3);
|
||||
expect(snap.seven_day).toBeUndefined();
|
||||
});
|
||||
|
||||
it('clear() drops all entries', () => {
|
||||
const store = freshStore();
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.5 });
|
||||
store.clear();
|
||||
expect(store.size).toBe(0);
|
||||
expect(store.get('five_hour')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('isApiKeyAuth', () => {
|
||||
it('matches verbose getAuthMethodDescription() output', () => {
|
||||
expect(isApiKeyAuth('API key (from ~/.claude-mem/.env)')).toBe(true);
|
||||
expect(isApiKeyAuth('Claude Code OAuth token (read from system keychain at spawn)')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches concise tokens', () => {
|
||||
expect(isApiKeyAuth('api_key')).toBe(true);
|
||||
expect(isApiKeyAuth('cli')).toBe(false);
|
||||
expect(isApiKeyAuth('')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldAbortForQuota — api_key auth', () => {
|
||||
let store: RateLimitStore;
|
||||
beforeEach(() => {
|
||||
store = freshStore();
|
||||
});
|
||||
|
||||
it('never aborts even at five_hour utilization 0.99', () => {
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.99, status: 'allowed_warning' });
|
||||
const decision = shouldAbortForQuota('api_key', store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('never aborts even at seven_day_opus 0.99', () => {
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.99 });
|
||||
const decision = shouldAbortForQuota('API key (from ~/.claude-mem/.env)', store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('never aborts when reset is imminent', () => {
|
||||
store.set({
|
||||
rateLimitType: 'five_hour',
|
||||
utilization: 0.92,
|
||||
resetsAt: FIXED_NOW + 60_000, // 1 min away
|
||||
});
|
||||
const decision = shouldAbortForQuota('api_key', store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldAbortForQuota — cli/oauth auth', () => {
|
||||
const cliAuth = 'Claude Code OAuth token (read from system keychain at spawn)';
|
||||
let store: RateLimitStore;
|
||||
beforeEach(() => {
|
||||
store = freshStore();
|
||||
});
|
||||
|
||||
it('aborts on five_hour at 0.96 with reason mentioning "five_hour"', () => {
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.96 });
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(true);
|
||||
expect(decision.window).toBe('five_hour');
|
||||
expect(decision.reason).toContain('five_hour');
|
||||
});
|
||||
|
||||
it('does not abort on five_hour at 0.94 (below 0.95 threshold, no reset pressure)', () => {
|
||||
store.set({
|
||||
rateLimitType: 'five_hour',
|
||||
utilization: 0.94,
|
||||
resetsAt: FIXED_NOW + 60 * 60 * 1000, // 1h away
|
||||
});
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('aborts on seven_day_opus at 0.94 (>= 0.93 threshold)', () => {
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.94 });
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(true);
|
||||
expect(decision.window).toBe('seven_day_opus');
|
||||
});
|
||||
|
||||
it('aborts on seven_day_sonnet at 0.93 (>= 0.92 threshold)', () => {
|
||||
store.set({ rateLimitType: 'seven_day_sonnet', utilization: 0.93 });
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(true);
|
||||
expect(decision.window).toBe('seven_day_sonnet');
|
||||
});
|
||||
|
||||
it('aborts on five_hour at 0.90 with resetsAt 10 min away (grace buffer)', () => {
|
||||
store.set({
|
||||
rateLimitType: 'five_hour',
|
||||
utilization: 0.90,
|
||||
resetsAt: FIXED_NOW + 10 * 60 * 1000, // 10 min
|
||||
});
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(true);
|
||||
expect(decision.window).toBe('five_hour');
|
||||
expect(decision.reason).toContain('resets');
|
||||
});
|
||||
|
||||
it('does not abort on five_hour at 0.90 with resetsAt 30 min away (outside grace)', () => {
|
||||
store.set({
|
||||
rateLimitType: 'five_hour',
|
||||
utilization: 0.90,
|
||||
resetsAt: FIXED_NOW + 30 * 60 * 1000, // 30 min
|
||||
});
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('does not abort when all windows are below threshold', () => {
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.5 });
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.4 });
|
||||
store.set({ rateLimitType: 'seven_day_sonnet', utilization: 0.3 });
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('skips reset-grace check when utilization is below the floor', () => {
|
||||
// resetsAt within grace window but util well below the 0.85 floor —
|
||||
// no point aborting on a window that just reset.
|
||||
store.set({
|
||||
rateLimitType: 'five_hour',
|
||||
utilization: 0.10,
|
||||
resetsAt: FIXED_NOW + 5 * 60 * 1000,
|
||||
});
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
|
||||
it('reports the first matching window when multiple are over threshold', () => {
|
||||
store.set({ rateLimitType: 'five_hour', utilization: 0.99 });
|
||||
store.set({ rateLimitType: 'seven_day_opus', utilization: 0.99 });
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(true);
|
||||
// five_hour is checked first per the iteration order.
|
||||
expect(decision.window).toBe('five_hour');
|
||||
});
|
||||
|
||||
it('does not abort with empty store', () => {
|
||||
const decision = shouldAbortForQuota(cliAuth, store, FIXED_NOW);
|
||||
expect(decision.abort).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user