From 92c36e90c2d7eb42d8219fb2c2608a4e15a2b808 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Fri, 24 Jul 2026 14:14:36 +0200 Subject: [PATCH 01/26] feat: show session and weekly usage limits in status bar (fork-issue-35) Adds a fork-issue-27 context indicator to the status bar, together with the Claude subscription usage: five-hour window and weekly utilization percentages with reset times, fetched from the unofficial OAuth usage endpoint using the CLI's own OAuth token, throttled to one request per five minutes and failing silently (stale or hidden display) on any error. A new rate_limit_event case additionally captures the five-hour reset time for free from the CLI stream. Co-Authored-By: Claude Fable 5 --- src/extension.ts | 153 +++++++++++++++++++++++++++++++++++++++++++++++ src/script.ts | 84 ++++++++++++++++++++++---- 2 files changed, 226 insertions(+), 11 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 8fa37fb..2631755 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -14,6 +14,15 @@ let OPENCREDITS_API_URL = 'https://ccc.api.opencredits.ai'; let OPENCREDITS_WEB_URL = 'https://ccc.opencredits.ai'; let OPENCREDITS_PUBLISHABLE_KEY = 'oc_pk_c43da4f9a9484ae484ad29bc97cc354f'; +// Undocumented endpoint for session-usage / weekly-limit percentages (#35). The +// server responds 429 to requests without a recognized User-Agent, hence pinning +// one that matches a real claude-code CLI release. +const USAGE_URL = 'https://api.anthropic.com/api/oauth/usage'; +const USAGE_USER_AGENT = 'claude-code/2.1.218'; + +// Base URL substrings that identify a known first-party endpoint (OpenCredits/router) +const KNOWN_ENDPOINT_MARKERS = ['opencredits.ai', 'localhost:8787']; + const exec = util.promisify(cp.exec); // Storage for diff content (used by DiffContentProvider) @@ -166,6 +175,14 @@ class ClaudeChatProvider { private _totalTokensOutput: number = 0; private _requestCount: number = 0; private _subscriptionType: string | undefined; // 'pro', 'max', or undefined for API users + // Session-usage / weekly-limit snapshot from the undocumented oauth/usage endpoint + // (#35), shown next to the #27 context indicator. Account-wide, not session-scoped + // — deliberately not reset in _newSession()/sessionCleared. + private _usageLimits: { fiveHour?: { pct: number; resetsAt?: number }, week?: { pct: number; resetsAt?: number } } | undefined = undefined; + private _usageLastFetchMs = 0; + // Fallback resetsAt for the five-hour window, learned from the CLI's own + // stream-json rate-limit events when the usage endpoint's resets_at is absent. + private _lastRateLimitResetsAt: number | undefined; private _accountInfoFetchedThisSession: boolean = false; // Track if we fetched account info this session private _pendingModelAfterPayment: string | null = null; private _currentSessionId: string | undefined; @@ -403,6 +420,9 @@ class ClaudeChatProvider { }); } + // Send (possibly cached) session-usage / weekly-limit percentages (#35) + void this._maybeSendUsageLimits(); + // Send platform information to webview this._sendPlatformInfo(); @@ -1612,12 +1632,28 @@ class ClaudeChatProvider { } }); + // #35: refresh session-usage / weekly-limit percentages alongside the + // existing totals update (throttled internally to 5 minutes). + void this._maybeSendUsageLimits(); + // Refresh OpenCredits balance after each request if using OpenCredits if (this._isOpenCredits() || this._getOpenCreditsKey()) { this._sendOpenCreditsBalance(); } } break; + + case 'rate_limit_event': { + // #35: learn the five-hour window's reset time from the CLI's own + // rate-limit events, as a fallback for when the usage endpoint's + // response doesn't include one for that window. + const rateLimitType = jsonData.rate_limit_info?.rateLimitType; + if (!rateLimitType || rateLimitType === 'five_hour') { + this._lastRateLimitResetsAt = jsonData.rate_limit_info?.resetsAt; + } + void this._maybeSendUsageLimits(); + break; + } } } @@ -3609,6 +3645,123 @@ class ClaudeChatProvider { }); } + // Reads the CLI's OAuth access token from ~/.claude/.credentials.json for the + // undocumented usage endpoint (#35). Read-only: never touches refreshToken, never + // logs the token, never sends it to the webview. Any failure (file missing, parse + // error) yields null. + private async _readOAuthAccessToken(): Promise { + try { + const homeDir = process.env.HOME || process.env.USERPROFILE || ''; + const credentialsPath = path.join(homeDir, '.claude', '.credentials.json'); + const content = await vscode.workspace.fs.readFile(vscode.Uri.file(credentialsPath)); + const parsed = JSON.parse(new TextDecoder().decode(content)); + return parsed?.claudeAiOauth?.accessToken ?? null; + } catch { + return null; + } + } + + // Fetch session-usage / weekly-limit percentages from the undocumented oauth/usage + // endpoint (#35). Best-effort: any failure (missing token, network error, + // unexpected response shape) yields null instead of throwing, so the caller can + // keep serving a stale cache. + private async _fetchUsageLimits(): Promise { + const token = await this._readOAuthAccessToken(); + if (!token) { + return null; + } + + try { + const response = await fetch(USAGE_URL, { + method: 'GET', + headers: { + 'Authorization': 'Bearer ' + token, + 'anthropic-beta': 'oauth-2025-04-20', + 'User-Agent': USAGE_USER_AGENT + } + }); + + if (!response.ok) { + this._permLog(`usageLimits fetch status=${response.status} hasData=false`); + return null; + } + + const data = await response.json() as any; + + // Parses one usage window (five_hour / seven_day). Drops the window + // entirely unless it has a valid numeric percentage; resets_at may be a + // unix-seconds number or an ISO string, anything else is left out. + const parseWindow = (win: any, isFiveHour: boolean): { pct: number; resetsAt?: number } | undefined => { + if (!win || typeof win !== 'object') { + return undefined; + } + const pct = win.utilization ?? win.used_percentage; + if (typeof pct !== 'number' || !isFinite(pct)) { + return undefined; + } + + let resetsAt: number | undefined; + const rawResetsAt = win.resets_at; + if (typeof rawResetsAt === 'number' && isFinite(rawResetsAt)) { + resetsAt = rawResetsAt; + } else if (typeof rawResetsAt === 'string') { + const parsedMs = Date.parse(rawResetsAt); + if (!isNaN(parsedMs)) { + resetsAt = parsedMs / 1000; + } + } + if (resetsAt === undefined && isFiveHour) { + resetsAt = this._lastRateLimitResetsAt; + } + + return { pct, resetsAt }; + }; + + const result: typeof this._usageLimits = {}; + const fiveHour = parseWindow(data?.five_hour, true); + if (fiveHour) { + result.fiveHour = fiveHour; + } + const week = parseWindow(data?.seven_day, false); + if (week) { + result.week = week; + } + + const hasData = !!(result.fiveHour || result.week); + this._permLog(`usageLimits fetch status=${response.status} hasData=${hasData}`); + + return hasData ? result : null; + } catch { + return null; + } + } + + // Pushes a (possibly cached) usage-limits snapshot to the webview, throttled to at + // most one real fetch every 5 minutes (#35). Gated on subscription type: API and + // OpenCredits users have no session/weekly limits to show. + private async _maybeSendUsageLimits(): Promise { + if (!this._subscriptionType) { + return; + } + + if (Date.now() - this._usageLastFetchMs < 300000) { + if (this._usageLimits) { + this._postMessage({ type: 'usageLimits', data: this._usageLimits }); + } + return; + } + + this._usageLastFetchMs = Date.now(); + const u = await this._fetchUsageLimits(); + if (u) { + this._usageLimits = u; + } + + if (this._usageLimits) { + this._postMessage({ type: 'usageLimits', data: this._usageLimits }); + } + } + // Update the model configuration for the local router private _updateLocalRouterModel(model: string, tierModels?: { sonnet: string; opus: string; haiku: string }): void { setModelConfig({ diff --git a/src/script.ts b/src/script.ts index 4c949e2..78a7491 100644 --- a/src/script.ts +++ b/src/script.ts @@ -1029,6 +1029,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt let totalCost = 0; let totalTokensInput = 0; let totalTokensOutput = 0; + let currentContextTokens = 0; + let latestUsage = null; let requestCount = 0; let isProcessing = false; let requestStartTime = null; @@ -1062,6 +1064,64 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt vscode.postMessage({ type: 'viewUsage', usageType: usageType }); } + // Approximate context-window size per model, used to turn currentContextTokens + // into a percentage for the status bar (#27). Best-effort approximation, not the + // model's authoritative limit — router models use context_length from the + // recommended-models catalog. Native fable/opus/sonnet are the 1M-token variants + // per user decision (this setup runs on those); 'default' and unknown models + // fall back to a conservative 200K (underestimating only warns early). + function getContextWindow(model) { + const nativeWindows = { fable: 1000000, opus: 1000000, sonnet: 1000000, 'default': 200000 }; + if (nativeWindows[model]) { + return nativeWindows[model]; + } + const recommended = (window.__recommendedModels || []).find(function(m) { return m.id === model; }); + return (recommended && recommended.context_length) || 200000; + } + + // Builds the "Ctx 12,345 / ~200K (62%)" status-bar fragment, with a warning/ + // critical class once usage crosses 80%/95% (#27). Empty string when there's no + // context reading yet, so the status line looks exactly like before in that case. + function getContextIndicatorHtml() { + if (!currentContextTokens || currentContextTokens <= 0) { + return ''; + } + const win = getContextWindow(currentModel); + const pct = win > 0 ? Math.round((currentContextTokens / win) * 100) : 0; + const ctxClass = pct >= 95 ? ' class="ctx-crit"' : pct >= 80 ? ' class="ctx-warn"' : ''; + const winStr = win >= 1000000 ? \`\${Math.round(win / 1000000)}M\` : \`\${Math.round(win / 1000)}K\`; + return \` • Ctx \${currentContextTokens.toLocaleString()} / ~\${winStr} (\${pct}%)\`; + } + + // Builds the "5h 42% · Wo 18%" status-bar fragment (#35), same structure/escaping + // as the #27 Ctx indicator above. Empty string when there's no usage data yet. + function getUsageIndicatorHtml() { + if (!latestUsage) return ''; + const fiveHour = latestUsage.fiveHour; + const week = latestUsage.week; + if (!fiveHour && !week) return ''; + + const fiveHourPct = fiveHour ? Math.round(fiveHour.pct) : undefined; + const weekPct = week ? Math.round(week.pct) : undefined; + const maxPct = Math.max(fiveHourPct || 0, weekPct || 0); + const usageClass = maxPct >= 95 ? ' class="ctx-crit"' : maxPct >= 80 ? ' class="ctx-warn"' : ''; + + const titleParts = []; + if (fiveHour && fiveHour.resetsAt) { + titleParts.push(\`5h resets \${new Date(fiveHour.resetsAt * 1000).toLocaleTimeString()}\`); + } + if (week && week.resetsAt) { + titleParts.push(\`Week resets \${new Date(week.resetsAt * 1000).toLocaleString()}\`); + } + const titleAttr = titleParts.length ? \` title="\${titleParts.join(' · ')}"\` : ''; + + const fiveHourStr = fiveHour ? \`5h \${fiveHourPct}%\` : ''; + const weekStr = week ? \`Wo \${weekPct}%\` : ''; + const text = fiveHour && week ? \`\${fiveHourStr} · \${weekStr}\` : (fiveHourStr || weekStr); + + return \` • \${text}\`; + } + function updateStatusWithTotals() { if (isProcessing) { // While processing, show elapsed time (and tokens for non-OpenCredits users) @@ -1076,13 +1136,11 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // OpenCredits users: don't show tokens, just elapsed time statusText = \`Processing\${elapsedStr ? \` • \${elapsedStr}\` : ''}\`; } else { - // Regular users: show tokens and elapsed time - const totalTokens = totalTokensInput + totalTokensOutput; - const tokensStr = totalTokens > 0 ? - \`\${totalTokens.toLocaleString()} tokens\` : '0 tokens'; - statusText = \`Processing • \${tokensStr}\${elapsedStr ? \` • \${elapsedStr}\` : ''}\`; + // Regular users: show context usage and elapsed time (#27 — the + // context indicator replaced the old cumulative token sum here) + statusText = \`Processing\${getContextIndicatorHtml()}\${getUsageIndicatorHtml()}\${elapsedStr ? \` • \${elapsedStr}\` : ''}\`; } - updateStatus(statusText, 'processing'); + updateStatusHtml(statusText, 'processing'); } else { // When ready, show full info let usageStr; @@ -1113,12 +1171,10 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const requestStr = requestCount > 0 ? \`\${requestCount} requests\` : ''; statusText = \`Ready\${requestStr ? \` • \${requestStr}\` : ''} • \${usageStr}\`; } else { - // Regular users: show tokens, requests, and usage - const totalTokens = totalTokensInput + totalTokensOutput; - const tokensStr = totalTokens > 0 ? - \`\${totalTokens.toLocaleString()} tokens\` : '0 tokens'; + // Regular users: show context usage, requests, and usage (#27 — the + // context indicator replaced the old cumulative token sum here) const requestStr = requestCount > 0 ? \`\${requestCount} requests\` : ''; - statusText = \`Ready • \${tokensStr}\${requestStr ? \` • \${requestStr}\` : ''} • \${usageStr}\`; + statusText = \`Ready\${getContextIndicatorHtml()}\${getUsageIndicatorHtml()}\${requestStr ? \` • \${requestStr}\` : ''} • \${usageStr}\`; } updateStatusHtml(statusText, 'ready'); } @@ -3730,6 +3786,12 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt updateStatusWithTotals(); break; + case 'usageLimits': + // Store session-usage / weekly-limit snapshot (#35) and refresh the status bar + latestUsage = message.data || null; + updateStatusWithTotals(); + break; + case 'modelSwitching': // Model is being switched (router restarting) currentModel = message.model; From 773e71c2f5bddc91ab776a44d6698e9dc00a2478 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 11:27:09 +0200 Subject: [PATCH 02/26] fix: parse and show per-model weekly usage buckets (fork-issue-35) The usage indicator's _usageLimits type gains sevenDayOpus/sevenDaySonnet fields, and the oauth/usage response parser now reads the per-model-tier weekly buckets seven_day_opus (shown as Fable) and seven_day_sonnet through the existing defensive parseWindow, alongside the five-hour and week windows it already handled. getUsageIndicatorHtml renders both new buckets in the status-bar usage indicator when the account's usage response includes them. Co-Authored-By: Claude Fable 5 --- src/extension.ts | 23 +++++++++++++++++------ src/script.ts | 25 ++++++++++++++++++++----- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 2631755..c48bf38 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -177,8 +177,10 @@ class ClaudeChatProvider { private _subscriptionType: string | undefined; // 'pro', 'max', or undefined for API users // Session-usage / weekly-limit snapshot from the undocumented oauth/usage endpoint // (#35), shown next to the #27 context indicator. Account-wide, not session-scoped - // — deliberately not reset in _newSession()/sessionCleared. - private _usageLimits: { fiveHour?: { pct: number; resetsAt?: number }, week?: { pct: number; resetsAt?: number } } | undefined = undefined; + // — deliberately not reset in _newSession()/sessionCleared. sevenDayOpus/ + // sevenDaySonnet are the per-model-tier weekly buckets (CLI schema names, not + // display labels — the opus-lineage bucket covers the Fable model shown to users). + private _usageLimits: { fiveHour?: { pct: number; resetsAt?: number }, week?: { pct: number; resetsAt?: number }, sevenDayOpus?: { pct: number; resetsAt?: number }, sevenDaySonnet?: { pct: number; resetsAt?: number } } | undefined = undefined; private _usageLastFetchMs = 0; // Fallback resetsAt for the five-hour window, learned from the CLI's own // stream-json rate-limit events when the usage endpoint's resets_at is absent. @@ -3688,9 +3690,10 @@ class ClaudeChatProvider { const data = await response.json() as any; - // Parses one usage window (five_hour / seven_day). Drops the window - // entirely unless it has a valid numeric percentage; resets_at may be a - // unix-seconds number or an ISO string, anything else is left out. + // Parses one usage window (five_hour / seven_day / seven_day_opus / + // seven_day_sonnet). Drops the window entirely unless it has a valid + // numeric percentage; resets_at may be a unix-seconds number or an ISO + // string, anything else is left out. const parseWindow = (win: any, isFiveHour: boolean): { pct: number; resetsAt?: number } | undefined => { if (!win || typeof win !== 'object') { return undefined; @@ -3726,8 +3729,16 @@ class ClaudeChatProvider { if (week) { result.week = week; } + const sevenDayOpus = parseWindow(data?.seven_day_opus, false); + if (sevenDayOpus) { + result.sevenDayOpus = sevenDayOpus; + } + const sevenDaySonnet = parseWindow(data?.seven_day_sonnet, false); + if (sevenDaySonnet) { + result.sevenDaySonnet = sevenDaySonnet; + } - const hasData = !!(result.fiveHour || result.week); + const hasData = !!(result.fiveHour || result.week || result.sevenDayOpus || result.sevenDaySonnet); this._permLog(`usageLimits fetch status=${response.status} hasData=${hasData}`); return hasData ? result : null; diff --git a/src/script.ts b/src/script.ts index 78a7491..4b2a891 100644 --- a/src/script.ts +++ b/src/script.ts @@ -1093,17 +1093,24 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt return \` • Ctx \${currentContextTokens.toLocaleString()} / ~\${winStr} (\${pct}%)\`; } - // Builds the "5h 42% · Wo 18%" status-bar fragment (#35), same structure/escaping - // as the #27 Ctx indicator above. Empty string when there's no usage data yet. + // Builds the "5h 42% · Wo 18% · Fable 30%" status-bar fragment (#35), same + // structure/escaping as the #27 Ctx indicator above. Fable/Sonnet are the + // per-model weekly buckets (seven_day_opus/seven_day_sonnet); each renders + // only when the account's usage data actually includes it. Empty string when + // there's no usage data yet. function getUsageIndicatorHtml() { if (!latestUsage) return ''; const fiveHour = latestUsage.fiveHour; const week = latestUsage.week; - if (!fiveHour && !week) return ''; + const sevenDayOpus = latestUsage.sevenDayOpus; + const sevenDaySonnet = latestUsage.sevenDaySonnet; + if (!fiveHour && !week && !sevenDayOpus && !sevenDaySonnet) return ''; const fiveHourPct = fiveHour ? Math.round(fiveHour.pct) : undefined; const weekPct = week ? Math.round(week.pct) : undefined; - const maxPct = Math.max(fiveHourPct || 0, weekPct || 0); + const sevenDayOpusPct = sevenDayOpus ? Math.round(sevenDayOpus.pct) : undefined; + const sevenDaySonnetPct = sevenDaySonnet ? Math.round(sevenDaySonnet.pct) : undefined; + const maxPct = Math.max(fiveHourPct || 0, weekPct || 0, sevenDayOpusPct || 0, sevenDaySonnetPct || 0); const usageClass = maxPct >= 95 ? ' class="ctx-crit"' : maxPct >= 80 ? ' class="ctx-warn"' : ''; const titleParts = []; @@ -1113,11 +1120,19 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt if (week && week.resetsAt) { titleParts.push(\`Week resets \${new Date(week.resetsAt * 1000).toLocaleString()}\`); } + if (sevenDayOpus && sevenDayOpus.resetsAt) { + titleParts.push(\`Fable resets \${new Date(sevenDayOpus.resetsAt * 1000).toLocaleString()}\`); + } + if (sevenDaySonnet && sevenDaySonnet.resetsAt) { + titleParts.push(\`Sonnet resets \${new Date(sevenDaySonnet.resetsAt * 1000).toLocaleString()}\`); + } const titleAttr = titleParts.length ? \` title="\${titleParts.join(' · ')}"\` : ''; const fiveHourStr = fiveHour ? \`5h \${fiveHourPct}%\` : ''; const weekStr = week ? \`Wo \${weekPct}%\` : ''; - const text = fiveHour && week ? \`\${fiveHourStr} · \${weekStr}\` : (fiveHourStr || weekStr); + const sevenDayOpusStr = sevenDayOpus ? \`Fable \${sevenDayOpusPct}%\` : ''; + const sevenDaySonnetStr = sevenDaySonnet ? \`Sonnet \${sevenDaySonnetPct}%\` : ''; + const text = [fiveHourStr, weekStr, sevenDayOpusStr, sevenDaySonnetStr].filter(Boolean).join(' · '); return \` • \${text}\`; } From 1bf5746ec678d3cf5f9baf75bfc2cad53ebbea0b Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 16:21:03 +0200 Subject: [PATCH 03/26] fix: read the per-model weekly window from the limits array (fork-issue-35) The oauth/usage endpoint now returns per-model weekly windows only as limits[] entries (kind "weekly_scoped" with a model scope, field "percent", 0-100 scale); the legacy seven_day_opus/seven_day_sonnet fields are null. Fall back to those entries, preferring is_active ones. Co-Authored-By: Claude Fable 5 --- src/extension.ts | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index c48bf38..68a6be9 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3691,14 +3691,15 @@ class ClaudeChatProvider { const data = await response.json() as any; // Parses one usage window (five_hour / seven_day / seven_day_opus / - // seven_day_sonnet). Drops the window entirely unless it has a valid - // numeric percentage; resets_at may be a unix-seconds number or an ISO - // string, anything else is left out. + // seven_day_sonnet, or a limits[] entry, which uses `percent` instead of + // `utilization`/`used_percentage`). Drops the window entirely unless it + // has a valid numeric percentage; resets_at may be a unix-seconds number + // or an ISO string, anything else is left out. const parseWindow = (win: any, isFiveHour: boolean): { pct: number; resetsAt?: number } | undefined => { if (!win || typeof win !== 'object') { return undefined; } - const pct = win.utilization ?? win.used_percentage; + const pct = win.utilization ?? win.used_percentage ?? win.percent; if (typeof pct !== 'number' || !isFinite(pct)) { return undefined; } @@ -3738,6 +3739,28 @@ class ClaudeChatProvider { result.sevenDaySonnet = sevenDaySonnet; } + // #35: newer accounts return the per-model weekly windows only as + // limits[] entries (kind "weekly_scoped" with a model scope) while the + // legacy seven_day_opus/seven_day_sonnet fields stay null. Top-level + // fields win when both are present. + if (Array.isArray(data?.limits)) { + // is_active entries first, so a stale scoped window cannot shadow + // the live one if several model-scoped entries are present. + const scoped = data.limits.filter((e: any) => e && e.kind === 'weekly_scoped'); + scoped.sort((a: any, b: any) => (b?.is_active === true ? 1 : 0) - (a?.is_active === true ? 1 : 0)); + for (const entry of scoped) { + const displayName = entry.scope?.model?.display_name; + if (typeof displayName !== 'string') { continue; } + const win = parseWindow(entry, false); + if (!win) { continue; } + if (/sonnet/i.test(displayName)) { + if (!result.sevenDaySonnet) { result.sevenDaySonnet = win; } + } else if (!result.sevenDayOpus) { + result.sevenDayOpus = win; + } + } + } + const hasData = !!(result.fiveHour || result.week || result.sevenDayOpus || result.sevenDaySonnet); this._permLog(`usageLimits fetch status=${response.status} hasData=${hasData}`); From f4e7cc99f2bdc44ea1dd0381a93901a32c806dd6 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 19:29:09 +0200 Subject: [PATCH 04/26] feat: add pure path/binary/uri helpers for the turn diff view (fork-issue-38) WSL-to-Windows path mapping, case-insensitive workspace-relative path computation (undefined for the root itself and anything outside), NUL-based binary detection and the deterministic claude-diff URI build/parse pair. vscode-free so the 21-case mocha suite (test:diff-utils) runs headless. Co-Authored-By: Claude Fable 5 --- src/diff-utils.ts | 116 ++++++++++++++++++++++++++++++++ src/test/diff-utils.test.ts | 129 ++++++++++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 src/diff-utils.ts create mode 100644 src/test/diff-utils.test.ts diff --git a/src/diff-utils.ts b/src/diff-utils.ts new file mode 100644 index 0000000..2798f52 --- /dev/null +++ b/src/diff-utils.ts @@ -0,0 +1,116 @@ +// Pure helpers for the #38 turn-diff feature (real vscode.diff view comparing the +// pre-turn checkpoint against the live file). No vscode import, so these run under +// plain mocha like shell-utils/auto-model-switch/model-updater -- extension.ts owns +// all the side effects (git exec, workspace lookup, vscode.Uri/vscode.diff) and just +// feeds paths/buffers through these functions. + +// Maps a WSL-reported path (e.g. /mnt/c/Users/Roman/foo.ts, as seen in tool_use +// rawInput.file_path when claudeCodeChat.wsl.enabled is on) back to the real Windows +// path VS Code and git need. Only rewrites an actual /mnt//... path; anything +// else (already a Windows path, or a Linux path outside /mnt) is returned unchanged. +export function mapWslPathToWindows(filePath: string): string { + const match = filePath.match(/^\/mnt\/([a-zA-Z])\/(.*)$/); + if (!match) { + return filePath; + } + const drive = match[1].toUpperCase(); + const rest = match[2].replace(/\//g, '\\'); + return `${drive}:\\${rest}`; +} + +// Resolves an absolute file path to a path relative to the workspace root, using '/' +// separators so the result can be passed straight to `git show :` +// (git's tree-ish path syntax always uses '/', regardless of OS). Comparison is +// case-insensitive on Windows, where the filesystem is case-insensitive but tool +// input paths and the workspace folder path aren't guaranteed to agree on casing. +// Returns undefined when filePath isn't inside workspaceRoot ("not mappable"), which +// also covers filePath being the workspace root itself (opus review FIX 4: a +// directory has no checkpointed blob to diff against, so treat it the same as +// "outside the workspace" instead of handing callers a '' relPath). +export function toWorkspaceRelativePath(filePath: string, workspaceRoot: string): string | undefined { + const normalize = (p: string) => p.replace(/\\/g, '/').replace(/\/+$/, ''); + const normFile = normalize(filePath); + const normRoot = normalize(workspaceRoot); + if (!normFile || !normRoot) { + return undefined; + } + + const caseInsensitive = process.platform === 'win32'; + const fileKey = caseInsensitive ? normFile.toLowerCase() : normFile; + const rootKey = caseInsensitive ? normRoot.toLowerCase() : normRoot; + + if (fileKey === rootKey) { + return undefined; + } + if (fileKey.startsWith(rootKey + '/')) { + return normFile.slice(normRoot.length + 1); + } + return undefined; +} + +// Binary heuristic used to keep obviously-binary content out of the diff virtual +// document: a NUL byte within the first 8 KB, the same window common tools (git, +// grep) use. Not exact binary detection -- just a "should we even try to diff this" +// guard before handing content to a text-based diff view. +const BINARY_CHECK_WINDOW_BYTES = 8192; + +export function isBinaryContent(content: Buffer): boolean { + const len = Math.min(content.length, BINARY_CHECK_WINDOW_BYTES); + for (let i = 0; i < len; i++) { + if (content[i] === 0) { + return true; + } + } + return false; +} + +// Scheme of the existing read-only diff content provider (registered once in +// extension.ts, reused here instead of adding a second provider). +export const TURN_DIFF_URI_SCHEME = 'claude-diff'; + +export interface TurnDiffUriParts { + scheme: string; + path: string; + query: string; +} + +// Builds the (scheme, path, query) a stable baseline URI is made of: deterministic +// per (sha, relPath), so vscode.Uri.from(parts) always produces the identical URI +// for the same turn+file and VS Code dedupes the tab instead of stacking a new one +// per click. relPath keeps its real extension in `path` (git's basename) so the +// virtual document still gets the right syntax highlighting; `sha` goes in the query +// so two turns diffing the same file don't collide on the same URI/cache entry. +export function buildTurnDiffUriParts(sha: string, relPath: string): TurnDiffUriParts { + return { + scheme: TURN_DIFF_URI_SCHEME, + path: '/' + relPath.replace(/^\/+/, ''), + query: `sha=${sha}` + }; +} + +// Inverse of buildTurnDiffUriParts (opus review FIX 3): recovers (sha, relPath) from +// a claude-diff URI's own (path, query), so DiffContentProvider can resolve a cache +// miss -- a tab restored via "Reopen Closed Editor" or a VS Code restart, after the +// in-memory diffContentStore is gone -- without needing any other state. vscode.Uri +// hands back path/query already decoded, matching what buildTurnDiffUriParts wrote, +// so this is a plain string split, not URI-decoding. Returns undefined when query +// doesn't look like a URI this feature built (defensive; shouldn't happen for a URI +// on the claude-diff scheme). +export function parseTurnDiffUriParts(parts: { path: string; query: string }): { sha: string; relPath: string } | undefined { + const match = parts.query.match(/^sha=(.*)$/); + if (!match) { + return undefined; + } + return { + sha: match[1], + relPath: parts.path.replace(/^\/+/, '') + }; +} + +// Cache key for the content-provider's Map, derived identically on the writer (host, +// right before vscode.diff) and reader (provideTextDocumentContent) side so a +// (sha, relPath) pair never collides with a different turn's baseline for the same +// file (see buildTurnDiffUriParts). +export function turnDiffCacheKey(parts: { path: string; query: string }): string { + return `${parts.path}?${parts.query}`; +} diff --git a/src/test/diff-utils.test.ts b/src/test/diff-utils.test.ts new file mode 100644 index 0000000..0b1a0fe --- /dev/null +++ b/src/test/diff-utils.test.ts @@ -0,0 +1,129 @@ +// Unit tests for the #38 turn-diff helpers (WSL path mapping, workspace-relative +// path resolution, binary detection, baseline URI construction). All pure (no +// vscode, no network, no filesystem access), so these run under plain mocha against +// the compiled out/ output -- same pattern as the shell-utils/auto-model-switch unit +// tests. Run with `npm run test:diff-utils`. + +import * as assert from 'assert'; +import { + mapWslPathToWindows, + toWorkspaceRelativePath, + isBinaryContent, + buildTurnDiffUriParts, + parseTurnDiffUriParts, + turnDiffCacheKey, + TURN_DIFF_URI_SCHEME +} from '../diff-utils'; + +suite('diff-utils: mapWslPathToWindows', () => { + + test('maps /mnt/c/... to C:\\...', () => { + assert.strictEqual(mapWslPathToWindows('/mnt/c/Users/Roman/foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + }); + + test('maps other drive letters too (e.g. /mnt/d)', () => { + assert.strictEqual(mapWslPathToWindows('/mnt/d/projects/bar.ts'), 'D:\\projects\\bar.ts'); + }); + + test('is case-insensitive on the drive letter and normalizes it to uppercase', () => { + assert.strictEqual(mapWslPathToWindows('/mnt/C/Users/Roman/foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + }); + + test('leaves an already-Windows path unchanged', () => { + assert.strictEqual(mapWslPathToWindows('C:\\Users\\Roman\\foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + }); + + test('leaves a non-/mnt Linux path unchanged (not a WSL-mapped drive)', () => { + assert.strictEqual(mapWslPathToWindows('/home/roman/foo.ts'), '/home/roman/foo.ts'); + }); +}); + +suite('diff-utils: toWorkspaceRelativePath', () => { + + test('resolves an exact match under the workspace root', () => { + assert.strictEqual(toWorkspaceRelativePath('C:\\proj\\src\\a.ts', 'C:\\proj'), 'src/a.ts'); + }); + + test('is case-insensitive (Windows paths)', () => { + assert.strictEqual(toWorkspaceRelativePath('c:\\PROJ\\src\\a.ts', 'C:\\proj'), 'src/a.ts'); + }); + + test('normalizes mixed \\ and / separators', () => { + assert.strictEqual(toWorkspaceRelativePath('C:/proj\\src/a.ts', 'C:\\proj'), 'src/a.ts'); + }); + + test('returns undefined for a file outside the workspace', () => { + assert.strictEqual(toWorkspaceRelativePath('C:\\other\\a.ts', 'C:\\proj'), undefined); + }); + + test('returns undefined when the path is the workspace root itself', () => { + assert.strictEqual(toWorkspaceRelativePath('C:\\proj', 'C:\\proj'), undefined); + }); +}); + +suite('diff-utils: isBinaryContent', () => { + + test('plain text is not binary', () => { + assert.strictEqual(isBinaryContent(Buffer.from('hello world\nline two\n', 'utf8')), false); + }); + + test('a NUL byte within the first 8 KB is detected as binary', () => { + const buf = Buffer.concat([Buffer.from('abc'), Buffer.from([0]), Buffer.from('def')]); + assert.strictEqual(isBinaryContent(buf), true); + }); + + test('a NUL byte after the first 8 KB is not detected (window limit)', () => { + const buf = Buffer.concat([Buffer.alloc(8200, 'a'), Buffer.from([0])]); + assert.strictEqual(isBinaryContent(buf), false); + }); + + test('an empty buffer is not binary', () => { + assert.strictEqual(isBinaryContent(Buffer.alloc(0)), false); + }); +}); + +suite('diff-utils: buildTurnDiffUriParts / turnDiffCacheKey', () => { + + test('is deterministic for the same (sha, relPath)', () => { + const a = buildTurnDiffUriParts('abc123', 'src/a.ts'); + const b = buildTurnDiffUriParts('abc123', 'src/a.ts'); + assert.deepStrictEqual(a, b); + assert.strictEqual(turnDiffCacheKey(a), turnDiffCacheKey(b)); + }); + + test('differs by sha for the same relPath', () => { + const a = buildTurnDiffUriParts('abc123', 'src/a.ts'); + const b = buildTurnDiffUriParts('def456', 'src/a.ts'); + assert.notStrictEqual(turnDiffCacheKey(a), turnDiffCacheKey(b)); + }); + + test('differs by relPath for the same sha', () => { + const a = buildTurnDiffUriParts('abc123', 'src/a.ts'); + const b = buildTurnDiffUriParts('abc123', 'src/b.ts'); + assert.notStrictEqual(turnDiffCacheKey(a), turnDiffCacheKey(b)); + }); + + test('uses the existing claude-diff scheme and keeps the real basename in the path', () => { + const parts = buildTurnDiffUriParts('abc123', 'src/a.ts'); + assert.strictEqual(parts.scheme, TURN_DIFF_URI_SCHEME); + assert.strictEqual(parts.scheme, 'claude-diff'); + assert.strictEqual(parts.path, '/src/a.ts'); + }); +}); + +suite('diff-utils: parseTurnDiffUriParts', () => { + + test('is the exact inverse of buildTurnDiffUriParts', () => { + const built = buildTurnDiffUriParts('abc123', 'src/a.ts'); + assert.deepStrictEqual(parseTurnDiffUriParts(built), { sha: 'abc123', relPath: 'src/a.ts' }); + }); + + test('round-trips a nested relPath', () => { + const built = buildTurnDiffUriParts('def456', 'src/sub/dir/file.tsx'); + assert.deepStrictEqual(parseTurnDiffUriParts(built), { sha: 'def456', relPath: 'src/sub/dir/file.tsx' }); + }); + + test('returns undefined when query has no sha= prefix (not a URI this feature built)', () => { + assert.strictEqual(parseTurnDiffUriParts({ path: '/src/a.ts', query: 'other=x' }), undefined); + }); +}); From d7249d60d335b95a400412fd571a0b294a49cbfb Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 19:29:09 +0200 Subject: [PATCH 05/26] feat: show code changes as a real VS Code diff against the turn checkpoint (fork-issue-38) Replaces the racy fileContentBefore diff (button hidden on tool_result, one-slot pending data, per-click Date.now tabs, global renderSideBySide mutation, dead openDiffByIndex path) with vscode.diff against the shadow backup repo: left side is the checkpoint before the turn (git show via execFile, 16MB buffer, 2MB/binary guards, BOM strip), right side is the real editable file; stable per-(sha,path) URIs dedupe tabs and the provider re-resolves restored tabs itself. The Open Diff button stays visible permanently (dataset-based, works after history load via the persisted showRestoreOption sha). Auto-opens once per file per turn after a successful Edit/MultiEdit/Write with preserveFocus, silently skipping files outside the workspace (scratchpad/memory edits are routine); gitignored paths never fake an all-new baseline. Configurable via claudeCodeChat.diff.autoOpen (default on) in settings and the modal. Co-Authored-By: Claude Fable 5 --- README.md | 2 +- package.json | 8 +- src/extension.ts | 385 +++++++++++++++++++++++++++++++++++------------ src/script.ts | 111 +++++--------- src/ui.ts | 8 + 5 files changed, 341 insertions(+), 173 deletions(-) diff --git a/README.md b/README.md index 7466283..d4e1df4 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Ditch the command line and experience Claude Code like never before. This extens ### 📝 **Inline Diff Viewer** - **Full Diff Display** - See complete file changes directly in Edit, MultiEdit, and Write messages -- **Open in VS Code Diff** - One-click button to open VS Code's native side-by-side diff editor +- **Open in VS Code Diff** - One-click button opens a real VS Code diff comparing the checkpoint from before the current turn against the live, editable file; it stays available after the edit completes and after reloading a saved conversation, and can auto-open after every successful edit (configurable in settings) - **Smart Truncation** - Long diffs are truncated with an expand button for better readability - **Syntax Highlighting** - Proper code highlighting in diff views - **Visual Change Indicators** - Clear green/red highlighting for additions and deletions diff --git a/package.json b/package.json index 6bcdf89..5569153 100644 --- a/package.json +++ b/package.json @@ -208,6 +208,11 @@ "type": "boolean", "default": false, "description": "Enable the local router to convert OpenAI format to Anthropic format. Required for providers that use OpenAI-compatible APIs." + }, + "claudeCodeChat.diff.autoOpen": { + "type": "boolean", + "default": true, + "description": "Automatically open a VS Code diff view after Claude successfully edits or creates a file, comparing it against the checkpoint from before the current turn." } } } @@ -221,7 +226,8 @@ "test": "vscode-test", "test:downloader": "npm run compile && mocha --ui tdd \"out/test/downloader*.test.js\" --reporter spec --timeout 360000", "test:downloader:unit": "npm run compile && mocha --ui tdd out/test/downloader.test.js --reporter spec", - "test:models": "npm run compile && mocha --ui tdd out/test/model-updater.test.js --reporter spec" + "test:models": "npm run compile && mocha --ui tdd out/test/model-updater.test.js --reporter spec", + "test:diff-utils": "npm run compile && mocha --ui tdd out/test/diff-utils.test.js --reporter spec" }, "devDependencies": { "@types/mocha": "^10.0.10", diff --git a/src/extension.ts b/src/extension.ts index 68a6be9..26fb7a0 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3,11 +3,13 @@ import * as cp from 'child_process'; import * as util from 'util'; import * as path from 'path'; import * as os from 'os'; +import * as fs from 'fs'; import getHtml from './ui'; import { startRouter, stopRouter, setModelConfig, setBaseUrl } from './router'; import { fetchAndResolveModels } from './model-updater'; import recommendedModels from './recommended-models.json'; import { downloadClaude, detectPlatform, DownloaderError } from './claudeDownloader'; +import { mapWslPathToWindows, toWorkspaceRelativePath, isBinaryContent, buildTurnDiffUriParts, parseTurnDiffUriParts, turnDiffCacheKey } from './diff-utils'; // OpenCredits environment configuration let OPENCREDITS_API_URL = 'https://ccc.api.opencredits.ai'; @@ -24,18 +26,85 @@ const USAGE_USER_AGENT = 'claude-code/2.1.218'; const KNOWN_ENDPOINT_MARKERS = ['opencredits.ai', 'localhost:8787']; const exec = util.promisify(cp.exec); - -// Storage for diff content (used by DiffContentProvider) +// Used only for the #38 turn-diff `git show` call: relPath is derived from a tool's +// file_path (Claude-controlled), so it goes through execFile's argv array instead of +// exec's shell string -- no shell means embedded quotes/metacharacters in a path can't +// break out into a second command, unlike the pre-existing exec() checkpoint calls +// below (untouched, out of scope here) which only ever see either a fixed argv, a sha +// git already produced itself, or this._backupRepoPath/workspacePath. +const execFile = util.promisify(cp.execFile); + +// File target for [perm] diagnostics (#15): console.error of an installed +// extension is only visible in the DevTools console, which makes field +// debugging of the stdio permission channel impossible — mirror it to a file. +const PERM_LOG_FILE = path.join(os.tmpdir(), 'claude-code-chat-perm.log'); + +// Storage for diff content (used by DiffContentProvider). Keyed by turnDiffCacheKey() +// (path+query) so two turns diffing the same relPath under different checkpoint SHAs +// don't collide on the same entry. Bounded (opus review FIX 3): entries used to be +// removed by an onDidCloseTextDocument listener, which neither fired for every tab +// lifecycle (e.g. vscode.diff throwing after the entry was already stored) nor could +// ever help resolve a cache miss -- a tab restored via "Reopen Closed Editor" or a VS +// Code restart starts with an empty store no listener could have populated. Since +// DiffContentProvider now resolves misses itself instead (see below), there's nothing +// left that needs a close-time delete; FIFO eviction here just caps how much stale +// baseline text can pile up from an unlucky sequence of turns. +const TURN_DIFF_CACHE_MAX_ENTRIES = 32; const diffContentStore = new Map(); -// Custom TextDocumentContentProvider for read-only diff views +function cacheTurnDiffContent(key: string, content: string): void { + if (diffContentStore.size >= TURN_DIFF_CACHE_MAX_ENTRIES) { + const oldestKey = diffContentStore.keys().next().value; + if (oldestKey !== undefined) { + diffContentStore.delete(oldestKey); + } + } + diffContentStore.set(key, content); +} + +// Custom TextDocumentContentProvider for read-only diff views (#38 turn diff: serves +// the pre-turn checkpoint content as the left/baseline side of vscode.diff). Content +// is normally already cached (written by _openTurnDiff right before vscode.diff is +// invoked), but a cache miss -- e.g. a claude-diff tab restored via "Reopen Closed +// Editor" or after a VS Code restart, see opus review FIX 3 -- is resolved on demand +// through the injected resolver, using only the (sha, relPath) already baked into the +// URI itself (see parseTurnDiffUriParts), so the provider needs no other state. class DiffContentProvider implements vscode.TextDocumentContentProvider { - provideTextDocumentContent(uri: vscode.Uri): string { - const content = diffContentStore.get(uri.path); - return content || ''; + constructor(private readonly _resolveBaseline: (sha: string, relPath: string) => Promise) { } + + async provideTextDocumentContent(uri: vscode.Uri): Promise { + const key = turnDiffCacheKey({ path: uri.path, query: uri.query }); + const cached = diffContentStore.get(key); + if (cached !== undefined) { + return cached; + } + + const parts = parseTurnDiffUriParts({ path: uri.path, query: uri.query }); + if (!parts) { + throw new Error('Claude turn baseline unavailable for this tab'); + } + try { + const content = await this._resolveBaseline(parts.sha, parts.relPath); + cacheTurnDiffContent(key, content); + return content; + } catch { + // Reason (git error, guard, no workspace/checkpoint repo) is intentionally + // not surfaced here -- VS Code just needs an honest "this tab has no + // content" error instead of a silent empty page; _openTurnDiff's own + // fallback path (manual toast / auto permLog) is what actually explains + // failures for the live open-diff flow. + throw new Error('Claude turn baseline unavailable for this tab'); + } } } +// #38 turn diff guards: `git show` is capped at a generous hard limit so a huge +// checkpointed file can't hang/OOM the exec call, but anything still over the much +// smaller display limit (or binary) falls back to opening the file directly instead +// of stuffing megabytes of text into a virtual document. +const TURN_DIFF_MAX_DISPLAY_BYTES = 2 * 1024 * 1024; +const TURN_DIFF_MAX_EXEC_BYTES = 16 * 1024 * 1024; + export function activate(context: vscode.ExtensionContext) { if (context.extensionMode === vscode.ExtensionMode.Development) { @@ -57,8 +126,11 @@ export function activate(context: vscode.ExtensionContext) { const webviewProvider = new ClaudeChatWebviewProvider(context.extensionUri, provider); vscode.window.registerWebviewViewProvider('claude-code-chat.chat', webviewProvider); - // Register custom content provider for read-only diff views - const diffProvider = new DiffContentProvider(); + // Register custom content provider for read-only diff views. Wired to the primary + // provider's baseline resolver (opus review FIX 3) -- extra panels from "New Claude + // Chat (Separate)" (#24) share the same extension context, so they resolve to the + // same backup repo anyway; a claude-diff tab has no panel of its own to route to. + const diffProvider = new DiffContentProvider((sha, relPath) => provider.resolveTurnDiffBaselineForProvider(sha, relPath)); context.subscriptions.push(vscode.workspace.registerTextDocumentContentProvider('claude-diff', diffProvider)); // Listen for configuration changes @@ -190,6 +262,10 @@ class ClaudeChatProvider { private _currentSessionId: string | undefined; private _backupRepoPath: string | undefined; private _commits: Array<{ id: string, sha: string, message: string, timestamp: string }> = []; + // #38 turn diff auto-open: files already auto-diffed in the current turn, so + // repeated edits to the same file don't keep reopening/refocusing the tab. Reset + // at the start of every turn in _sendMessageToClaude. + private _autoOpenedDiffFilesThisTurn: Set = new Set(); private _conversationsPath: string | undefined; // Pending permission requests from stdio control_request messages private _pendingPermissionRequests: Map(); + // Clear draft message since we're sending it this._draftMessage = ''; @@ -1514,7 +1605,8 @@ class ClaudeChatProvider { const isError = content.is_error || false; // Find the last tool use to get the tool name, input, and computed startLine - const lastToolUse = this._currentConversation[this._currentConversation.length - 1] + const toolUseMessageIndex = this._currentConversation.length - 1; + const lastToolUse = this._currentConversation[toolUseMessageIndex]; const toolName = lastToolUse?.data?.toolName; const rawInput = lastToolUse?.data?.rawInput; @@ -1563,6 +1655,23 @@ class ClaudeChatProvider { } }); } + + // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write, + // once per file per turn (see _autoOpenedDiffFilesThisTurn reset in + // _sendMessageToClaude). Manual "Open Diff" clicks go through the same + // _openTurnDiff but aren't gated by the setting or this dedup set. + // trigger: 'auto' (opus review FIX 1) -- Claude sessions routinely edit + // files outside the workspace (scratchpad, ~/.claude memory, etc.), so + // _openTurnDiff failing here is the ordinary case, not something to + // interrupt the user with a toast/focus-stealing showTextDocument for. + if ((toolName === 'Edit' || toolName === 'MultiEdit' || toolName === 'Write') && !isError && rawInput?.file_path) { + const autoOpenDiff = vscode.workspace.getConfiguration('claudeCodeChat').get('diff.autoOpen', true); + const dedupeKey = process.platform === 'win32' ? rawInput.file_path.toLowerCase() : rawInput.file_path; + if (autoOpenDiff && !this._autoOpenedDiffFilesThisTurn.has(dedupeKey)) { + this._autoOpenedDiffFilesThisTurn.add(dedupeKey); + void this._openTurnDiff(rawInput.file_path, toolUseMessageIndex, 'auto'); + } + } } } } @@ -3438,6 +3547,7 @@ class ClaudeChatProvider { 'executable.path': config.get('executable.path', ''), 'environment.variables': config.get>('environment.variables', {}), 'environment.disabled': config.get('environment.disabled', false), + 'diff.autoOpen': config.get('diff.autoOpen', true), 'isOpenCredits': this._isOpenCredits() }; @@ -4064,106 +4174,189 @@ class ClaudeChatProvider { } } - private async _openDiffByMessageIndex(messageIndex: number) { - try { - const message = this._currentConversation[messageIndex]; - if (!message) { - console.error('Message not found at index:', messageIndex); - return; + // #38 turn diff: walks _currentConversation backwards from messageIndex (inclusive) + // to the nearest showRestoreOption entry, which is the checkpoint commit made right + // before this turn's user message (_createBackupCommit runs before every turn). Works + // both live and after a history reload -- unlike _commits (#50), _currentConversation + // is exactly what gets persisted/reloaded, so the index lines up either way. + private _findTurnBaselineSha(messageIndex: number): string | undefined { + const start = Math.min(messageIndex, this._currentConversation.length - 1); + for (let i = start; i >= 0; i--) { + const entry = this._currentConversation[i]; + if (entry?.messageType === 'showRestoreOption' && entry.data?.sha) { + return entry.data.sha; } + } + return undefined; + } - const data = message.data; - const toolName = data.toolName; - const rawInput = data.rawInput; - let filePath = rawInput?.file_path || ''; - let oldContent = ''; - let newContent = ''; - - if (!filePath) { - console.error('No file path found for message at index:', messageIndex); - return; - } + // Shared failure path for every way _openTurnDiff can come up short (no checkpoint, + // git error, file outside the workspace/not WSL-mappable, too large/binary baseline): + // never fail silently for a real user click -- tell them why there's no diff and + // open the real file instead so a click is never a dead end. `trigger` (opus + // review FIX 1) tells 'manual' (webview "Open Diff" button, a deliberate user + // action -- toast + focus is fine) apart from 'auto' (post tool_result auto-open, + // see the Edit/MultiEdit/Write handler above): Claude sessions routinely edit files + // outside the workspace (scratchpad, ~/.claude memory, etc.), so failing here is + // the ordinary case for auto-open, not something worth a toast/focus-stealing + // showTextDocument for -- it only gets a permLog line for field diagnostics. + private async _openTurnDiffFallback(filePath: string, trigger: 'manual' | 'auto', reason: string): Promise { + if (trigger === 'auto') { + // First line only: git error messages can be multi-line and would break the + // one-line-per-entry perm-log format. + this._permLog(`[turndiff] auto skip reason=${reason.split('\n')[0]} file=${filePath}`); + return; + } + vscode.window.showInformationMessage(`Claude Code Chat: ${reason}; showing the file instead.`); + try { + await vscode.window.showTextDocument(vscode.Uri.file(filePath)); + } catch (error) { + console.error('Failed to open fallback file for turn diff:', error); + } + } - // Read current file from disk - this is the "before" state since edit hasn't been applied yet - try { - const fileUri = vscode.Uri.file(filePath); - const fileData = await vscode.workspace.fs.readFile(fileUri); - oldContent = Buffer.from(fileData).toString('utf8'); - } catch { - // File might not exist yet (for Write creating new file) - oldContent = ''; + // opus review FIX 2: distinguishes a genuinely new file (nothing existed at the + // checkpoint yet) from a file that's simply gitignored in the shadow backup repo + // (_createBackupCommit's `add -A` silently skips ignored paths) -- both produce the + // identical `does not exist in ` from `git show`, but only the first should + // get an empty "new file" baseline. --git-dir/--work-tree matches the existing + // checkpoint calls (_initializeBackupRepo/_createBackupCommit above). `check-ignore + // -q` exits 0 when the path IS ignored; per git's own docs it exits 1 (an execFile + // rejection, not a bug) when it's NOT ignored, which is the common case. + private async _isPathIgnoredInBackupRepo(backupRepoPath: string, workTreePath: string, relPath: string): Promise { + try { + // cwd pinned to the work tree: git resolves the relative path against the + // process cwd's prefix inside the work tree, so an unpinned cwd would make + // anchored .gitignore entries (like /out/) match or miss depending on where + // the extension host happens to run (opus delta-review). + await execFile('git', ['--git-dir', backupRepoPath, '--work-tree', workTreePath, 'check-ignore', '-q', '--', relPath], { cwd: workTreePath }); + return true; + } catch (error: any) { + if (error?.code === 1) { + return false; } + // Anything else (git missing, fatal error, ...): can't confirm either way, + // so let the caller fail closed instead of risking a wrong empty baseline. + throw error; + } + } - // Compute "after" state by applying the edit to current file - if (toolName === 'Edit' && rawInput?.old_string && rawInput?.new_string) { - newContent = oldContent.replace(rawInput.old_string, rawInput.new_string); - } else if (toolName === 'MultiEdit' && rawInput?.edits) { - newContent = oldContent; - for (const edit of rawInput.edits) { - if (edit.old_string && edit.new_string) { - newContent = newContent.replace(edit.old_string, edit.new_string); - } + // Reads the checkpointed blob for relPath at sha from the shadow backup repo and + // returns it as a UTF-8 string, or throws when there's nothing sane to show. Shared + // by _openTurnDiff (manual/auto "open diff", already knows workspaceFolder/sha from + // the live call) and resolveTurnDiffBaselineForProvider (a DiffContentProvider + // cache miss, opus review FIX 3) so both go through the identical git-show + + // classification + guards, and BOM-stripping only has to happen in one place + // (opus review FIX 5). Never returns a silently-wrong baseline -- callers each + // decide what "failure" means for their UI (fallback toast/permLog vs. a generic + // VS Code tab error). + private async _resolveTurnDiffBaseline(backupRepoPath: string, workTreePath: string, sha: string, relPath: string): Promise { + let content: Buffer; + try { + const { stdout } = await execFile( + 'git', + ['--git-dir', backupRepoPath, 'show', `${sha}:${relPath}`], + { encoding: 'buffer', maxBuffer: TURN_DIFF_MAX_EXEC_BYTES } + ); + content = stdout; + } catch (error: any) { + const stderrText = Buffer.isBuffer(error?.stderr) ? error.stderr.toString('utf8') : String(error?.stderr || error?.message || ''); + // opus review FIX 2: `exists on disk, but not in ` is deliberately NOT + // treated as "new file" below. Best effort only: whether git emits that + // message (vs. plain `does not exist in`) depends on the process cwd seeing + // the on-disk file, so e.g. a case-only mismatch (Src/ vs src/) is not + // reliably caught -- but when the message does appear, an empty baseline + // would silently lie, so it must go down the failure path. + if (/does not exist in/i.test(stderrText)) { + let ignored: boolean; + try { + ignored = await this._isPathIgnoredInBackupRepo(backupRepoPath, workTreePath, relPath); + } catch (ignoreError: any) { + throw new Error(`failed to read the checkpoint (${ignoreError.message})`); } - } else if (toolName === 'Write' && rawInput?.content) { - newContent = rawInput.content; - } - - if (oldContent !== newContent) { - await this._openDiffEditor(oldContent, newContent, filePath); + if (ignored) { + throw new Error('file is not tracked by checkpoints (excluded via .gitignore)'); + } + // Genuinely new file: nothing existed at the checkpoint, so the + // baseline is empty and the whole file shows as added. + content = Buffer.alloc(0); } else { - vscode.window.showInformationMessage('No changes to show - the edit may have already been applied.'); + throw new Error(`failed to read the checkpoint (${error.message})`); } - } catch (error) { - console.error('Error opening diff by message index:', error); } + + if (content.length > TURN_DIFF_MAX_DISPLAY_BYTES || isBinaryContent(content)) { + throw new Error('file is too large or binary to diff'); + } + + // VS Code strips the BOM from the real file's text model, keep both sides + // consistent (opus review FIX 5). + return content.toString('utf8').replace(/^\uFEFF/, ''); } - private async _openDiffEditor(oldContent: string, newContent: string, filePath: string) { - try { - // oldContent and newContent are now full file contents passed from the webview - const baseName = path.basename(filePath); - const timestamp = Date.now(); + // Public seam for DiffContentProvider's injected resolver (opus review FIX 3, + // wired up in activate()) -- reuses the same backup-repo baseline lookup + // _openTurnDiff uses, keyed only by the (sha, relPath) already encoded in a + // claude-diff tab's own URI, so a tab restored via "Reopen Closed Editor" or a VS + // Code restart can resolve itself without any per-turn state. Errors are left for + // the caller (DiffContentProvider) to fold into its single generic tab error. + public async resolveTurnDiffBaselineForProvider(sha: string, relPath: string): Promise { + const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; + if (!workspaceFolder || !this._backupRepoPath) { + throw new Error('no workspace or checkpoint repository available'); + } + return this._resolveTurnDiffBaseline(this._backupRepoPath, workspaceFolder.uri.fsPath, sha, relPath); + } - // Create unique paths for the virtual documents - const oldPath = `/${timestamp}/old/${baseName}`; - const newPath = `/${timestamp}/new/${baseName}`; + // #38: opens a real VS Code diff -- the checkpoint from right before this turn + // (left, read-only virtual document served from the shadow backup repo via + // DiffContentProvider) against the actual file on disk (right, live/editable, so + // later edits in the same turn keep showing up in the same tab). Shared by the + // manual "Open Diff" button and the auto-open after a successful tool_result; + // `trigger` picks which of the two _openTurnDiffFallback behaves as (opus review + // FIX 1). + private async _openTurnDiff(filePath: string, messageIndex: number, trigger: 'manual' | 'auto'): Promise { + const resolvedPath = mapWslPathToWindows(filePath); - // Store content in the global store for the content provider - diffContentStore.set(oldPath, oldContent); - diffContentStore.set(newPath, newContent); + const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; + if (!workspaceFolder || !this._backupRepoPath) { + await this._openTurnDiffFallback(resolvedPath, trigger, 'no workspace or checkpoint repository available'); + return; + } - // Create URIs with our custom scheme - const oldUri = vscode.Uri.parse(`claude-diff:${oldPath}`); - const newUri = vscode.Uri.parse(`claude-diff:${newPath}`); + const sha = this._findTurnBaselineSha(messageIndex); + if (!sha) { + await this._openTurnDiffFallback(resolvedPath, trigger, 'no checkpoint found for this turn'); + return; + } - // Ensure side-by-side diff mode is enabled - const diffConfig = vscode.workspace.getConfiguration('diffEditor'); - const wasInlineMode = diffConfig.get('renderSideBySide') === false; - if (wasInlineMode) { - await diffConfig.update('renderSideBySide', true, vscode.ConfigurationTarget.Global); - } + // toWorkspaceRelativePath also returns undefined when resolvedPath IS the + // workspace root itself (opus review FIX 4) -- a directory has no checkpointed + // blob to diff against, so it's handled the same as "outside the workspace". + const relPath = toWorkspaceRelativePath(resolvedPath, workspaceFolder.uri.fsPath); + if (relPath === undefined) { + await this._openTurnDiffFallback(resolvedPath, trigger, 'file is outside the workspace'); + return; + } - // Open diff editor - await vscode.commands.executeCommand('vscode.diff', oldUri, newUri, `${baseName} (Changes)`); + let content: string; + try { + content = await this._resolveTurnDiffBaseline(this._backupRepoPath, workspaceFolder.uri.fsPath, sha, relPath); + } catch (error: any) { + await this._openTurnDiffFallback(resolvedPath, trigger, error.message); + return; + } - // Clean up stored content when documents are closed - const closeListener = vscode.workspace.onDidCloseTextDocument((doc) => { - if (doc.uri.toString() === oldUri.toString()) { - diffContentStore.delete(oldPath); - } - if (doc.uri.toString() === newUri.toString()) { - diffContentStore.delete(newPath); - } - // Dispose listener when both are cleaned up - if (!diffContentStore.has(oldPath) && !diffContentStore.has(newPath)) { - closeListener.dispose(); - } - }); + try { + const uriParts = buildTurnDiffUriParts(sha, relPath); + const baselineUri = vscode.Uri.from(uriParts); + cacheTurnDiffContent(turnDiffCacheKey(uriParts), content); - this._disposables.push(closeListener); - } catch (error) { - vscode.window.showErrorMessage(`Failed to open diff editor: ${error}`); - console.error('Error opening diff editor:', error); + const rightUri = vscode.Uri.file(resolvedPath); + const title = `${path.basename(resolvedPath)} (Turn Diff)`; + await vscode.commands.executeCommand('vscode.diff', baselineUri, rightUri, title, { preserveFocus: true }); + } catch (error: any) { + await this._openTurnDiffFallback(resolvedPath, trigger, `failed to open the diff view (${error.message})`); } } diff --git a/src/script.ts b/src/script.ts index 4b2a891..237553e 100644 --- a/src/script.ts +++ b/src/script.ts @@ -79,20 +79,19 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt let planModeEnabled = false; let thinkingModeEnabled = false; let isWindows = false; - let lastPendingEditIndex = -1; // Track the last Edit/MultiEdit/Write toolUse without result - let lastPendingEditData = null; // Store diff data for the pending edit { filePath, oldContent, newContent } let attachedImages = []; // Array of { filePath, previewUri } - // Open diff using stored data (no file read needed) - function openDiffEditor() { - if (lastPendingEditData) { - vscode.postMessage({ - type: 'openDiff', - filePath: lastPendingEditData.filePath, - oldContent: lastPendingEditData.oldContent, - newContent: lastPendingEditData.newContent - }); - } + // #38: request a real VS Code diff (checkpoint-before-turn vs. the live file) for + // one Edit/MultiEdit/Write message. filePath/messageIndex come from the clicked + // button's own dataset (see generateUnifiedDiffHTML/formatMultiEditToolDiff), not + // a shared pending-edit slot, so the button keeps working after tool_result and + // after a history reload. + function requestTurnDiff(filePath, messageIndex) { + vscode.postMessage({ + type: 'openTurnDiff', + filePath: filePath, + messageIndex: parseInt(messageIndex, 10) + }); } function shouldAutoScroll(messagesDiv) { @@ -244,50 +243,18 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Format raw input with expandable content for long values // Use diff format for Edit, MultiEdit, and Write tools, regular format for others if (data.toolName === 'Edit' || data.toolName === 'MultiEdit' || data.toolName === 'Write') { - // Only show Open Diff button if we have fileContentBefore (live session, not reload) - const showButton = data.fileContentBefore !== undefined && data.messageIndex >= 0; - - // Hide any existing pending edit button before showing new one - if (showButton && lastPendingEditIndex >= 0) { - const prevContent = document.querySelector('[data-edit-message-index="' + lastPendingEditIndex + '"]'); - if (prevContent) { - const btn = prevContent.querySelector('.diff-open-btn'); - if (btn) btn.style.display = 'none'; - } - lastPendingEditData = null; - } - - if (showButton) { - lastPendingEditIndex = data.messageIndex; - contentDiv.setAttribute('data-edit-message-index', data.messageIndex); - - // Compute and store diff data for when button is clicked - const oldContent = data.fileContentBefore || ''; - let newContent = oldContent; - if (data.toolName === 'Edit' && data.rawInput.old_string && data.rawInput.new_string) { - newContent = oldContent.replace(data.rawInput.old_string, data.rawInput.new_string); - } else if (data.toolName === 'MultiEdit' && data.rawInput.edits) { - for (const edit of data.rawInput.edits) { - if (edit.old_string && edit.new_string) { - newContent = newContent.replace(edit.old_string, edit.new_string); - } - } - } else if (data.toolName === 'Write' && data.rawInput.content) { - newContent = data.rawInput.content; - } - lastPendingEditData = { - filePath: data.rawInput.file_path, - oldContent: oldContent, - newContent: newContent - }; - } + // #38: the Open Diff button stays visible after tool_result and after a + // history reload -- it only needs a valid messageIndex (used to look up + // the pre-turn checkpoint on the host side), not the live-only, + // optimistic fileContentBefore read. + const showButton = data.messageIndex >= 0; if (data.toolName === 'Edit') { - contentDiv.innerHTML = formatEditToolDiff(data.rawInput, data.fileContentBefore, showButton, data.startLine); + contentDiv.innerHTML = formatEditToolDiff(data.rawInput, data.fileContentBefore, showButton, data.startLine, data.messageIndex); } else if (data.toolName === 'MultiEdit') { - contentDiv.innerHTML = formatMultiEditToolDiff(data.rawInput, data.fileContentBefore, showButton, data.startLines); + contentDiv.innerHTML = formatMultiEditToolDiff(data.rawInput, data.fileContentBefore, showButton, data.startLines, data.messageIndex); } else { - contentDiv.innerHTML = formatWriteToolDiff(data.rawInput, data.fileContentBefore, showButton); + contentDiv.innerHTML = formatWriteToolDiff(data.rawInput, data.fileContentBefore, showButton, data.messageIndex); } } else if (data.toolName === 'ExitPlanMode' && data.rawInput) { contentDiv.innerHTML = formatPlanOutput(data.rawInput); @@ -350,20 +317,9 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const messagesDiv = document.getElementById('messages'); const shouldScroll = shouldAutoScroll(messagesDiv); - // When result comes in for Edit/MultiEdit/Write, hide the Open Diff button on the request - // since the edit has now been applied (no longer pending) - if (lastPendingEditIndex >= 0) { - // Find and hide the button on the corresponding toolUse - const toolUseContent = document.querySelector('[data-edit-message-index="' + lastPendingEditIndex + '"]'); - if (toolUseContent) { - const btn = toolUseContent.querySelector('.diff-open-btn'); - if (btn) { - btn.style.display = 'none'; - } - } - lastPendingEditIndex = -1; - lastPendingEditData = null; - } + // #38: the Open Diff button on the request no longer gets hidden when its + // result arrives -- it stays available (and auto-open, if enabled, has + // already opened/updated the same turn diff by the time this runs). // For Read and TodoWrite tools, just hide loading state (no result message needed) if ((data.toolName === 'Read' || data.toolName === 'TodoWrite') && !data.isError) { @@ -606,7 +562,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Generate unified diff HTML with line numbers // showButton controls whether to show the "Open Diff" button - function generateUnifiedDiffHTML(oldString, newString, filePath, startLine = 1, showButton = false) { + function generateUnifiedDiffHTML(oldString, newString, filePath, startLine = 1, showButton = false, messageIndex = -1) { const oldLines = oldString.split('\\n'); const newLines = newString.split('\\n'); const diff = computeLineDiff(oldLines, newLines); @@ -709,7 +665,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt html += '
'; html += 'Summary: ' + summary + ''; if (showButton) { - html += ''; } @@ -719,7 +675,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt return html; } - function formatEditToolDiff(input, fileContentBefore, showButton = false, providedStartLine = null) { + function formatEditToolDiff(input, fileContentBefore, showButton = false, providedStartLine = null, messageIndex = -1) { if (!input || typeof input !== 'object') { return formatToolInputUI(input); } @@ -740,10 +696,10 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } } - return generateUnifiedDiffHTML(input.old_string, input.new_string, input.file_path, startLine, showButton); + return generateUnifiedDiffHTML(input.old_string, input.new_string, input.file_path, startLine, showButton, messageIndex); } - function formatMultiEditToolDiff(input, fileContentBefore, showButton = false, providedStartLines = null) { + function formatMultiEditToolDiff(input, fileContentBefore, showButton = false, providedStartLines = null, messageIndex = -1) { if (!input || typeof input !== 'object') { return formatToolInputUI(input); } @@ -808,7 +764,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt html += '
'; html += 'Summary: ' + input.edits.length + ' edit' + (input.edits.length > 1 ? 's' : '') + ''; if (showButton) { - html += ''; } @@ -817,7 +773,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt return html; } - function formatWriteToolDiff(input, fileContentBefore, showButton = false) { + function formatWriteToolDiff(input, fileContentBefore, showButton = false, messageIndex = -1) { if (!input || typeof input !== 'object') { return formatToolInputUI(input); } @@ -831,7 +787,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const fullFileBefore = fileContentBefore || ''; // Show full content as added lines (new file or replacement) - return generateUnifiedDiffHTML(fullFileBefore, input.content, input.file_path, 1, showButton); + return generateUnifiedDiffHTML(fullFileBefore, input.content, input.file_path, 1, showButton, messageIndex); } function escapeHtml(text) { @@ -4940,6 +4896,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const yoloMode = document.getElementById('yolo-mode').checked; const executablePath = document.getElementById('executable-path').value; const useRouter = document.getElementById('use-router')?.checked || false; + // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write + const diffAutoOpen = document.getElementById('diff-auto-open').checked; // Collect environment variables from key-value UI const envVariables = getEnvVariablesFromUI(); @@ -4979,7 +4937,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt 'permissions.yoloMode': yoloMode, 'executable.path': executablePath, 'environment.variables': envVariables, - 'router.enabled': useRouter + 'router.enabled': useRouter, + 'diff.autoOpen': diffAutoOpen } }); } @@ -5236,6 +5195,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt }); } else if (message.type === 'settingsData') { // Update UI with current settings + // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write + document.getElementById('diff-auto-open').checked = message.data['diff.autoOpen'] !== false; const thinkingIntensity = message.data['thinking.intensity'] || 'think'; const intensityValues = ['think', 'think-hard', 'think-harder', 'ultrathink']; const sliderValue = intensityValues.indexOf(thinkingIntensity); diff --git a/src/ui.ts b/src/ui.ts index 4ffcab7..e1452ea 100644 --- a/src/ui.ts +++ b/src/ui.ts @@ -414,6 +414,14 @@ const getHtml = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'https

+

Diff View

+
+
+ + +
+
+
From a6ade54fc05b26b7fe5e0536af51500697020423 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 20:21:21 +0200 Subject: [PATCH 06/26] fix: refresh usage limits right after a turn ends instead of serving the 5-minute cache (fork-issue-54) Co-Authored-By: Claude Fable 5 --- src/extension.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 26fb7a0..16eb8c6 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1743,8 +1743,14 @@ class ClaudeChatProvider { } }); - // #35: refresh session-usage / weekly-limit percentages alongside the - // existing totals update (throttled internally to 5 minutes). + // #35/#54: refresh session-usage / weekly-limit percentages alongside the + // existing totals update. The finished turn just consumed usage, so the + // 5-minute throttle would show stale percentages for exactly the update + // the user is watching — bypass it, with a 30s floor so rapid-fire turns + // don't hammer the undocumented endpoint. + if (Date.now() - this._usageLastFetchMs > 30000) { + this._usageLastFetchMs = 0; + } void this._maybeSendUsageLimits(); // Refresh OpenCredits balance after each request if using OpenCredits From 550ae12aaaf3a223a3395fb4b060b8f09d3e8abe Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Mon, 27 Jul 2026 05:59:56 +0200 Subject: [PATCH 07/26] fix: restore checkpoints after history load via backup-repo existence check (fork-issue-50) _commits is cleared when a history load switches conversations, but the replayed showRestoreOption messages keep their Restore buttons alive, so _restoreToCommit answered every click with "Commit not found". On a _commits miss, confirm the sha directly against the shadow backup repo (execFile git cat-file -e ^{commit}) and rehydrate the display info from the replayed entry; shas are format-validated before reaching any git command. Live-session behavior is unchanged, and a genuinely missing commit still reports "Commit not found". New pure helpers in restore-commit-utils.ts with 15 unit tests (test:restore-commit-utils). Co-Authored-By: Claude Fable 5 --- package.json | 3 +- src/extension.ts | 40 ++++++++- src/restore-commit-utils.ts | 47 +++++++++++ src/test/restore-commit-utils.test.ts | 112 ++++++++++++++++++++++++++ 4 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 src/restore-commit-utils.ts create mode 100644 src/test/restore-commit-utils.test.ts diff --git a/package.json b/package.json index 5569153..74d9676 100644 --- a/package.json +++ b/package.json @@ -227,7 +227,8 @@ "test:downloader": "npm run compile && mocha --ui tdd \"out/test/downloader*.test.js\" --reporter spec --timeout 360000", "test:downloader:unit": "npm run compile && mocha --ui tdd out/test/downloader.test.js --reporter spec", "test:models": "npm run compile && mocha --ui tdd out/test/model-updater.test.js --reporter spec", - "test:diff-utils": "npm run compile && mocha --ui tdd out/test/diff-utils.test.js --reporter spec" + "test:diff-utils": "npm run compile && mocha --ui tdd out/test/diff-utils.test.js --reporter spec", + "test:restore-commit-utils": "npm run compile && mocha --ui tdd out/test/restore-commit-utils.test.js --reporter spec" }, "devDependencies": { "@types/mocha": "^10.0.10", diff --git a/src/extension.ts b/src/extension.ts index 16eb8c6..22e05b5 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -10,6 +10,7 @@ import { fetchAndResolveModels } from './model-updater'; import recommendedModels from './recommended-models.json'; import { downloadClaude, detectPlatform, DownloaderError } from './claudeDownloader'; import { mapWslPathToWindows, toWorkspaceRelativePath, isBinaryContent, buildTurnDiffUriParts, parseTurnDiffUriParts, turnDiffCacheKey } from './diff-utils'; +import { isValidCommitSha, findRehydratedCommitInfo } from './restore-commit-utils'; // OpenCredits environment configuration let OPENCREDITS_API_URL = 'https://ccc.api.opencredits.ai'; @@ -1965,7 +1966,44 @@ class ClaudeChatProvider { private async _restoreToCommit(commitSha: string): Promise { try { - const commit = this._commits.find(c => c.sha === commitSha); + // #50: commitSha can arrive rehydrated from a loaded conversation's + // persisted JSON, not only from same-session git output -- validate + // before it can reach any git command below. + if (!isValidCommitSha(commitSha)) { + this._postMessage({ + type: 'restoreError', + data: 'Commit not found' + }); + return; + } + + let commit = this._commits.find(c => c.sha === commitSha); + + // #50: a history load that switches conversations clears _commits but + // still replays this commit's showRestoreOption message, so its Restore + // button outlives this lookup. Confirm the sha against the shadow backup + // repo instead and rehydrate the display info from the replayed entry. + if (!commit && this._backupRepoPath) { + try { + // argv/no-shell (unlike the exec() calls below): commitSha can come + // from persisted JSON. `^{commit}` rejects a tree/blob sha that + // happens to pass the hex check -- still a single argv element. + await execFile('git', ['--git-dir', this._backupRepoPath, 'cat-file', '-e', `${commitSha}^{commit}`]); + commit = findRehydratedCommitInfo(this._currentConversation, commitSha); + } catch (error: any) { + // With the ^{commit} peel, git reports both "sha missing" and "sha + // not a commit" as exit 128 + "fatal: Not a valid object name" (not + // exit 1 like _isPathIgnoredInBackupRepo's plain cat-file), so + // classify on stderr like _resolveTurnDiffBaseline does: that text + // is the silent, expected miss; anything else (ENOENT, broken + // backup repo) is real infrastructure failure worth a log line. + const stderrText = String(error?.stderr || ''); + if (!/Not a valid object name/i.test(stderrText)) { + console.error('Failed to check commit existence in backup repo:', error.message); + } + } + } + if (!commit) { this._postMessage({ type: 'restoreError', diff --git a/src/restore-commit-utils.ts b/src/restore-commit-utils.ts new file mode 100644 index 0000000..81cbfe9 --- /dev/null +++ b/src/restore-commit-utils.ts @@ -0,0 +1,47 @@ +// Pure helpers for the #50 checkpoint-restore fix: after a history load switches +// conversations, extension.ts's in-memory _commits list is cleared (see +// _loadConversationHistory) even though the replayed showRestoreOption messages still +// show a working Restore button for a checkpoint that still exists in the shadow +// backup repo. No vscode import, so these run under plain mocha -- extension.ts owns +// all the side effects (the git cat-file -e existence check, the actual restore). + +export interface RestoreCommitInfo { + id: string; + sha: string; + message: string; + timestamp: string; +} + +// Every commit sha this extension itself ever produces comes straight from `git +// rev-parse HEAD` (trimmed), always plain lowercase hex -- 40 chars for SHA-1, 64 for +// SHA-256. commitSha can now also arrive here rehydrated from a loaded conversation's +// showRestoreOption entry, i.e. from persisted JSON on disk rather than only that +// same-session git output, so this must be checked before commitSha reaches any git +// command. 7 is the shortest abbreviation git itself would ever treat as unambiguous. +export function isValidCommitSha(sha: string): boolean { + return /^[0-9a-f]{7,64}$/i.test(sha); +} + +// Recovers a commit's display info (message/timestamp for the restore toasts) from the +// matching showRestoreOption entry replayed into _currentConversation, for a sha that +// _commits no longer knows about (#50: cleared by a history load that switched +// conversations). Only called once the caller has independently confirmed sha still +// exists in the backup repo -- this never claims a sha exists, only recovers its +// metadata, and falls back to a minimal placeholder built from the sha itself when no +// matching entry is found (e.g. an older saved conversation from before this field +// existed). +export function findRehydratedCommitInfo( + messages: ReadonlyArray<{ messageType: string, data: any }>, + sha: string +): RestoreCommitInfo { + const entry = messages.find(m => m.messageType === 'showRestoreOption' && m.data?.sha === sha); + if (entry && typeof entry.data?.message === 'string' && typeof entry.data?.timestamp === 'string') { + return { + id: typeof entry.data.id === 'string' ? entry.data.id : `commit-${sha}`, + sha, + message: entry.data.message, + timestamp: entry.data.timestamp + }; + } + return { id: `commit-${sha}`, sha, message: sha, timestamp: new Date().toISOString() }; +} diff --git a/src/test/restore-commit-utils.test.ts b/src/test/restore-commit-utils.test.ts new file mode 100644 index 0000000..aac5fe6 --- /dev/null +++ b/src/test/restore-commit-utils.test.ts @@ -0,0 +1,112 @@ +// Unit tests for the #50 checkpoint-restore fix (isValidCommitSha, +// findRehydratedCommitInfo). Pure (no vscode, no network, no filesystem access), so +// these run under plain mocha against the compiled out/ output -- same pattern as +// diff-utils/shell-utils/auto-model-switch. The actual "does this commit still exist" +// check (git cat-file -e against the shadow backup repo) stays in extension.ts, +// untested here, same as diff-utils' git-show baseline read. Run with +// `npm run test:restore-commit-utils`. + +import * as assert from 'assert'; +import { isValidCommitSha, findRehydratedCommitInfo } from '../restore-commit-utils'; + +suite('restore-commit-utils: isValidCommitSha', () => { + + test('a full 40-char SHA-1 (as produced by `git rev-parse HEAD`) is valid', () => { + assert.strictEqual(isValidCommitSha('a'.repeat(40)), true); + }); + + test('a full 64-char SHA-256 is valid', () => { + assert.strictEqual(isValidCommitSha('a'.repeat(64)), true); + }); + + test('is case-insensitive (uppercase hex is valid)', () => { + assert.strictEqual(isValidCommitSha('ABCDEF0123456789abcdef0123456789abcdef01'), true); + }); + + test('a 7-char abbreviation is valid (shortest git itself would treat as unambiguous)', () => { + assert.strictEqual(isValidCommitSha('abcdef1'), true); + }); + + test('shorter than 7 chars is rejected', () => { + assert.strictEqual(isValidCommitSha('abcde'), false); + }); + + test('longer than 64 chars is rejected', () => { + assert.strictEqual(isValidCommitSha('a'.repeat(65)), false); + }); + + test('empty string is rejected', () => { + assert.strictEqual(isValidCommitSha(''), false); + }); + + test('a shell-metacharacter injection attempt is rejected', () => { + assert.strictEqual(isValidCommitSha('deadbeef; rm -rf /'), false); + }); + + test('a sha with a trailing quote (attribute-breakout style) is rejected', () => { + assert.strictEqual(isValidCommitSha('deadbeef"'), false); + }); +}); + +suite('restore-commit-utils: findRehydratedCommitInfo', () => { + + const sha = 'deadbeef00112233445566778899aabbccddeeff'; + + test('rehydrates message/timestamp from the matching showRestoreOption entry', () => { + const messages = [ + { messageType: 'userInput', data: 'hi' }, + { messageType: 'showRestoreOption', data: { id: 'commit-1', sha, message: 'Before: fix bug', timestamp: '2026-07-27T10:00:00.000Z' } } + ]; + assert.deepStrictEqual(findRehydratedCommitInfo(messages, sha), { + id: 'commit-1', + sha, + message: 'Before: fix bug', + timestamp: '2026-07-27T10:00:00.000Z' + }); + }); + + test('ignores a showRestoreOption entry for a different sha', () => { + const messages = [ + { messageType: 'showRestoreOption', data: { id: 'commit-1', sha: 'other'.padEnd(40, '0'), message: 'Before: other', timestamp: '2026-07-27T10:00:00.000Z' } } + ]; + const result = findRehydratedCommitInfo(messages, sha); + assert.strictEqual(result.message, sha, 'must fall back to the placeholder, not the other entry'); + }); + + test('ignores a non-showRestoreOption message whose data coincidentally has a matching sha field', () => { + const messages = [ + { messageType: 'toolResult', data: { sha, message: 'not a checkpoint' } } + ]; + const result = findRehydratedCommitInfo(messages, sha); + assert.strictEqual(result.message, sha, 'must fall back to the placeholder, not the unrelated entry'); + }); + + test('falls back to id `commit-` when the matched entry has no id field', () => { + const messages = [ + { messageType: 'showRestoreOption', data: { sha, message: 'Before: fix bug', timestamp: '2026-07-27T10:00:00.000Z' } } + ]; + const result = findRehydratedCommitInfo(messages, sha); + assert.strictEqual(result.id, `commit-${sha}`); + assert.strictEqual(result.message, 'Before: fix bug'); + }); + + test('falls back to a sha-based placeholder when the matched entry has a non-string message (malformed data)', () => { + const messages = [ + { messageType: 'showRestoreOption', data: { sha, message: 42, timestamp: '2026-07-27T10:00:00.000Z' } } + ]; + const result = findRehydratedCommitInfo(messages, sha); + assert.strictEqual(result.message, sha); + assert.strictEqual(result.id, `commit-${sha}`); + }); + + test('falls back to a sha-based placeholder with a valid timestamp when no entry matches at all', () => { + const result = findRehydratedCommitInfo([], sha); + assert.deepStrictEqual(result, { + id: `commit-${sha}`, + sha, + message: sha, + timestamp: result.timestamp + }); + assert.notStrictEqual(new Date(result.timestamp).toString(), 'Invalid Date'); + }); +}); From 93aff1307e71a4eedea43ddf16b2e8bf213dbdc7 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 17:40:15 +0200 Subject: [PATCH 08/26] Fix stale Fable references in usage-limits comment and status line extension.ts's _usageLimits comment still described the opus-lineage bucket as covering "the Fable model shown to users" -- there is no Fable model in this branch (_setSelectedModel only accepts opus/sonnet/default), so that clause is just wrong; dropped it. Also fixes the usage-limits status line (src/script.ts): the German "Wo" abbreviation becomes "Week", and the sevenDayOpus bucket's "Fable" label becomes "Opus" (it's the Opus-tier weekly window, mirroring the existing "Sonnet" label for sevenDaySonnet) instead of naming a model that isn't selectable in this branch. Co-Authored-By: Claude Sonnet 5 --- src/extension.ts | 3 ++- src/script.ts | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 22e05b5..f3e53ae 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -252,7 +252,7 @@ class ClaudeChatProvider { // (#35), shown next to the #27 context indicator. Account-wide, not session-scoped // — deliberately not reset in _newSession()/sessionCleared. sevenDayOpus/ // sevenDaySonnet are the per-model-tier weekly buckets (CLI schema names, not - // display labels — the opus-lineage bucket covers the Fable model shown to users). + // display labels). private _usageLimits: { fiveHour?: { pct: number; resetsAt?: number }, week?: { pct: number; resetsAt?: number }, sevenDayOpus?: { pct: number; resetsAt?: number }, sevenDaySonnet?: { pct: number; resetsAt?: number } } | undefined = undefined; private _usageLastFetchMs = 0; // Fallback resetsAt for the five-hour window, learned from the CLI's own @@ -1769,6 +1769,7 @@ class ClaudeChatProvider { if (!rateLimitType || rateLimitType === 'five_hour') { this._lastRateLimitResetsAt = jsonData.rate_limit_info?.resetsAt; } + void this._maybeSendUsageLimits(); break; } diff --git a/src/script.ts b/src/script.ts index 237553e..ab37901 100644 --- a/src/script.ts +++ b/src/script.ts @@ -1049,8 +1049,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt return \` • Ctx \${currentContextTokens.toLocaleString()} / ~\${winStr} (\${pct}%)\`; } - // Builds the "5h 42% · Wo 18% · Fable 30%" status-bar fragment (#35), same - // structure/escaping as the #27 Ctx indicator above. Fable/Sonnet are the + // Builds the "5h 42% · Week 18% · Opus 30%" status-bar fragment (#35), same + // structure/escaping as the #27 Ctx indicator above. Opus/Sonnet are the // per-model weekly buckets (seven_day_opus/seven_day_sonnet); each renders // only when the account's usage data actually includes it. Empty string when // there's no usage data yet. @@ -1077,7 +1077,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt titleParts.push(\`Week resets \${new Date(week.resetsAt * 1000).toLocaleString()}\`); } if (sevenDayOpus && sevenDayOpus.resetsAt) { - titleParts.push(\`Fable resets \${new Date(sevenDayOpus.resetsAt * 1000).toLocaleString()}\`); + titleParts.push(\`Opus resets \${new Date(sevenDayOpus.resetsAt * 1000).toLocaleString()}\`); } if (sevenDaySonnet && sevenDaySonnet.resetsAt) { titleParts.push(\`Sonnet resets \${new Date(sevenDaySonnet.resetsAt * 1000).toLocaleString()}\`); @@ -1085,8 +1085,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const titleAttr = titleParts.length ? \` title="\${titleParts.join(' · ')}"\` : ''; const fiveHourStr = fiveHour ? \`5h \${fiveHourPct}%\` : ''; - const weekStr = week ? \`Wo \${weekPct}%\` : ''; - const sevenDayOpusStr = sevenDayOpus ? \`Fable \${sevenDayOpusPct}%\` : ''; + const weekStr = week ? \`Week \${weekPct}%\` : ''; + const sevenDayOpusStr = sevenDayOpus ? \`Opus \${sevenDayOpusPct}%\` : ''; const sevenDaySonnetStr = sevenDaySonnet ? \`Sonnet \${sevenDaySonnetPct}%\` : ''; const text = [fiveHourStr, weekStr, sevenDayOpusStr, sevenDaySonnetStr].filter(Boolean).join(' · '); @@ -5231,6 +5231,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update Customize Claude Command settings document.getElementById('executable-path').value = message.data['executable.path'] || ''; + renderEnvVariables(message.data['environment.variables'] || {}); // Detect OpenCredits and envs disabled state From f3d91a258bb642c82620b0d3602b6a888f1abfe8 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 19:30:20 +0200 Subject: [PATCH 09/26] Fix context-window sizes and scrub internal-fork references getContextWindow() claimed a 1M-token native window for opus/sonnet (and a fable entry that isn't even selectable in this branch) instead of the real 200K, so the status-bar Ctx percentage under-reported usage. Removed the fable entry and reset opus/sonnet to 200000. Also cleaned up references that only made sense inside the private fork: two comments pointed at shell-utils/auto-model-switch, a module already removed from this branch and one that never existed here; a few inline code-review shorthand notes were shortened; example paths using a real name became generic placeholders; and bare #NN issue markers now read fork-issue-NN so they aren't mistaken for this project's own issue tracker. Co-Authored-By: Claude Sonnet 5 --- src/diff-utils.ts | 14 +++--- src/extension.ts | 70 +++++++++++++-------------- src/restore-commit-utils.ts | 4 +- src/script.ts | 31 ++++++------ src/test/diff-utils.test.ts | 14 +++--- src/test/restore-commit-utils.test.ts | 4 +- 6 files changed, 68 insertions(+), 69 deletions(-) diff --git a/src/diff-utils.ts b/src/diff-utils.ts index 2798f52..81c9333 100644 --- a/src/diff-utils.ts +++ b/src/diff-utils.ts @@ -1,10 +1,10 @@ -// Pure helpers for the #38 turn-diff feature (real vscode.diff view comparing the +// Pure helpers for the fork-issue-38 turn-diff feature (real vscode.diff view comparing the // pre-turn checkpoint against the live file). No vscode import, so these run under -// plain mocha like shell-utils/auto-model-switch/model-updater -- extension.ts owns -// all the side effects (git exec, workspace lookup, vscode.Uri/vscode.diff) and just -// feeds paths/buffers through these functions. +// plain mocha like model-updater -- extension.ts owns all the side effects (git exec, +// workspace lookup, vscode.Uri/vscode.diff) and just feeds paths/buffers through +// these functions. -// Maps a WSL-reported path (e.g. /mnt/c/Users/Roman/foo.ts, as seen in tool_use +// Maps a WSL-reported path (e.g. /mnt/c/Users/example/foo.ts, as seen in tool_use // rawInput.file_path when claudeCodeChat.wsl.enabled is on) back to the real Windows // path VS Code and git need. Only rewrites an actual /mnt//... path; anything // else (already a Windows path, or a Linux path outside /mnt) is returned unchanged. @@ -24,7 +24,7 @@ export function mapWslPathToWindows(filePath: string): string { // case-insensitive on Windows, where the filesystem is case-insensitive but tool // input paths and the workspace folder path aren't guaranteed to agree on casing. // Returns undefined when filePath isn't inside workspaceRoot ("not mappable"), which -// also covers filePath being the workspace root itself (opus review FIX 4: a +// also covers filePath being the workspace root itself (review FIX 4: a // directory has no checkpointed blob to diff against, so treat it the same as // "outside the workspace" instead of handing callers a '' relPath). export function toWorkspaceRelativePath(filePath: string, workspaceRoot: string): string | undefined { @@ -88,7 +88,7 @@ export function buildTurnDiffUriParts(sha: string, relPath: string): TurnDiffUri }; } -// Inverse of buildTurnDiffUriParts (opus review FIX 3): recovers (sha, relPath) from +// Inverse of buildTurnDiffUriParts (review FIX 3): recovers (sha, relPath) from // a claude-diff URI's own (path, query), so DiffContentProvider can resolve a cache // miss -- a tab restored via "Reopen Closed Editor" or a VS Code restart, after the // in-memory diffContentStore is gone -- without needing any other state. vscode.Uri diff --git a/src/extension.ts b/src/extension.ts index f3e53ae..3cba698 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -17,7 +17,7 @@ let OPENCREDITS_API_URL = 'https://ccc.api.opencredits.ai'; let OPENCREDITS_WEB_URL = 'https://ccc.opencredits.ai'; let OPENCREDITS_PUBLISHABLE_KEY = 'oc_pk_c43da4f9a9484ae484ad29bc97cc354f'; -// Undocumented endpoint for session-usage / weekly-limit percentages (#35). The +// Undocumented endpoint for session-usage / weekly-limit percentages (fork-issue-35). The // server responds 429 to requests without a recognized User-Agent, hence pinning // one that matches a real claude-code CLI release. const USAGE_URL = 'https://api.anthropic.com/api/oauth/usage'; @@ -27,7 +27,7 @@ const USAGE_USER_AGENT = 'claude-code/2.1.218'; const KNOWN_ENDPOINT_MARKERS = ['opencredits.ai', 'localhost:8787']; const exec = util.promisify(cp.exec); -// Used only for the #38 turn-diff `git show` call: relPath is derived from a tool's +// Used only for the fork-issue-38 turn-diff `git show` call: relPath is derived from a tool's // file_path (Claude-controlled), so it goes through execFile's argv array instead of // exec's shell string -- no shell means embedded quotes/metacharacters in a path can't // break out into a second command, unlike the pre-existing exec() checkpoint calls @@ -35,14 +35,14 @@ const exec = util.promisify(cp.exec); // git already produced itself, or this._backupRepoPath/workspacePath. const execFile = util.promisify(cp.execFile); -// File target for [perm] diagnostics (#15): console.error of an installed +// File target for [perm] diagnostics (fork-issue-15): console.error of an installed // extension is only visible in the DevTools console, which makes field // debugging of the stdio permission channel impossible — mirror it to a file. const PERM_LOG_FILE = path.join(os.tmpdir(), 'claude-code-chat-perm.log'); // Storage for diff content (used by DiffContentProvider). Keyed by turnDiffCacheKey() // (path+query) so two turns diffing the same relPath under different checkpoint SHAs -// don't collide on the same entry. Bounded (opus review FIX 3): entries used to be +// don't collide on the same entry. Bounded (review FIX 3): entries used to be // removed by an onDidCloseTextDocument listener, which neither fired for every tab // lifecycle (e.g. vscode.diff throwing after the entry was already stored) nor could // ever help resolve a cache miss -- a tab restored via "Reopen Closed Editor" or a VS @@ -63,11 +63,11 @@ function cacheTurnDiffContent(key: string, content: string): void { diffContentStore.set(key, content); } -// Custom TextDocumentContentProvider for read-only diff views (#38 turn diff: serves +// Custom TextDocumentContentProvider for read-only diff views (fork-issue-38 turn diff: serves // the pre-turn checkpoint content as the left/baseline side of vscode.diff). Content // is normally already cached (written by _openTurnDiff right before vscode.diff is // invoked), but a cache miss -- e.g. a claude-diff tab restored via "Reopen Closed -// Editor" or after a VS Code restart, see opus review FIX 3 -- is resolved on demand +// Editor" or after a VS Code restart, see review FIX 3 -- is resolved on demand // through the injected resolver, using only the (sha, relPath) already baked into the // URI itself (see parseTurnDiffUriParts), so the provider needs no other state. class DiffContentProvider implements vscode.TextDocumentContentProvider { @@ -99,7 +99,7 @@ class DiffContentProvider implements vscode.TextDocumentContentProvider { } } -// #38 turn diff guards: `git show` is capped at a generous hard limit so a huge +// fork-issue-38 turn diff guards: `git show` is capped at a generous hard limit so a huge // checkpointed file can't hang/OOM the exec call, but anything still over the much // smaller display limit (or binary) falls back to opening the file directly instead // of stuffing megabytes of text into a virtual document. @@ -128,8 +128,8 @@ export function activate(context: vscode.ExtensionContext) { vscode.window.registerWebviewViewProvider('claude-code-chat.chat', webviewProvider); // Register custom content provider for read-only diff views. Wired to the primary - // provider's baseline resolver (opus review FIX 3) -- extra panels from "New Claude - // Chat (Separate)" (#24) share the same extension context, so they resolve to the + // provider's baseline resolver (review FIX 3) -- extra panels from "New Claude + // Chat (Separate)" (fork-issue-24) share the same extension context, so they resolve to the // same backup repo anyway; a claude-diff tab has no panel of its own to route to. const diffProvider = new DiffContentProvider((sha, relPath) => provider.resolveTurnDiffBaselineForProvider(sha, relPath)); context.subscriptions.push(vscode.workspace.registerTextDocumentContentProvider('claude-diff', diffProvider)); @@ -249,7 +249,7 @@ class ClaudeChatProvider { private _requestCount: number = 0; private _subscriptionType: string | undefined; // 'pro', 'max', or undefined for API users // Session-usage / weekly-limit snapshot from the undocumented oauth/usage endpoint - // (#35), shown next to the #27 context indicator. Account-wide, not session-scoped + // (fork-issue-35), shown next to the fork-issue-27 context indicator. Account-wide, not session-scoped // — deliberately not reset in _newSession()/sessionCleared. sevenDayOpus/ // sevenDaySonnet are the per-model-tier weekly buckets (CLI schema names, not // display labels). @@ -263,7 +263,7 @@ class ClaudeChatProvider { private _currentSessionId: string | undefined; private _backupRepoPath: string | undefined; private _commits: Array<{ id: string, sha: string, message: string, timestamp: string }> = []; - // #38 turn diff auto-open: files already auto-diffed in the current turn, so + // fork-issue-38 turn diff auto-open: files already auto-diffed in the current turn, so // repeated edits to the same file don't keep reopening/refocusing the tab. Reset // at the start of every turn in _sendMessageToClaude. private _autoOpenedDiffFilesThisTurn: Set = new Set(); @@ -320,7 +320,7 @@ class ClaudeChatProvider { } /** - * [perm] diagnostics (#15): mirror to console AND a temp file, because the + * [perm] diagnostics (fork-issue-15): mirror to console AND a temp file, because the * console of an installed extension host is not persisted anywhere readable. * Logging must never break the extension — swallow all fs errors. */ @@ -514,7 +514,7 @@ class ClaudeChatProvider { }); } - // Send (possibly cached) session-usage / weekly-limit percentages (#35) + // Send (possibly cached) session-usage / weekly-limit percentages (fork-issue-35) void this._maybeSendUsageLimits(); // Send platform information to webview @@ -1004,7 +1004,7 @@ class ClaudeChatProvider { this._isProcessing = true; - // #38 turn diff auto-open: fresh per-turn dedup set for this new turn. + // fork-issue-38 turn diff auto-open: fresh per-turn dedup set for this new turn. this._autoOpenedDiffFilesThisTurn = new Set(); // Clear draft message since we're sending it @@ -1657,11 +1657,11 @@ class ClaudeChatProvider { }); } - // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write, + // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write, // once per file per turn (see _autoOpenedDiffFilesThisTurn reset in // _sendMessageToClaude). Manual "Open Diff" clicks go through the same // _openTurnDiff but aren't gated by the setting or this dedup set. - // trigger: 'auto' (opus review FIX 1) -- Claude sessions routinely edit + // trigger: 'auto' (review FIX 1) -- Claude sessions routinely edit // files outside the workspace (scratchpad, ~/.claude memory, etc.), so // _openTurnDiff failing here is the ordinary case, not something to // interrupt the user with a toast/focus-stealing showTextDocument for. @@ -1744,7 +1744,7 @@ class ClaudeChatProvider { } }); - // #35/#54: refresh session-usage / weekly-limit percentages alongside the + // fork-issue-35/fork-issue-54: refresh session-usage / weekly-limit percentages alongside the // existing totals update. The finished turn just consumed usage, so the // 5-minute throttle would show stale percentages for exactly the update // the user is watching — bypass it, with a 30s floor so rapid-fire turns @@ -1762,7 +1762,7 @@ class ClaudeChatProvider { break; case 'rate_limit_event': { - // #35: learn the five-hour window's reset time from the CLI's own + // fork-issue-35: learn the five-hour window's reset time from the CLI's own // rate-limit events, as a fallback for when the usage endpoint's // response doesn't include one for that window. const rateLimitType = jsonData.rate_limit_info?.rateLimitType; @@ -1967,7 +1967,7 @@ class ClaudeChatProvider { private async _restoreToCommit(commitSha: string): Promise { try { - // #50: commitSha can arrive rehydrated from a loaded conversation's + // fork-issue-50: commitSha can arrive rehydrated from a loaded conversation's // persisted JSON, not only from same-session git output -- validate // before it can reach any git command below. if (!isValidCommitSha(commitSha)) { @@ -1980,7 +1980,7 @@ class ClaudeChatProvider { let commit = this._commits.find(c => c.sha === commitSha); - // #50: a history load that switches conversations clears _commits but + // fork-issue-50: a history load that switches conversations clears _commits but // still replays this commit's showRestoreOption message, so its Restore // button outlives this lookup. Confirm the sha against the shadow backup // repo instead and rehydrate the display info from the replayed entry. @@ -3803,7 +3803,7 @@ class ClaudeChatProvider { } // Reads the CLI's OAuth access token from ~/.claude/.credentials.json for the - // undocumented usage endpoint (#35). Read-only: never touches refreshToken, never + // undocumented usage endpoint (fork-issue-35). Read-only: never touches refreshToken, never // logs the token, never sends it to the webview. Any failure (file missing, parse // error) yields null. private async _readOAuthAccessToken(): Promise { @@ -3819,7 +3819,7 @@ class ClaudeChatProvider { } // Fetch session-usage / weekly-limit percentages from the undocumented oauth/usage - // endpoint (#35). Best-effort: any failure (missing token, network error, + // endpoint (fork-issue-35). Best-effort: any failure (missing token, network error, // unexpected response shape) yields null instead of throwing, so the caller can // keep serving a stale cache. private async _fetchUsageLimits(): Promise { @@ -3894,7 +3894,7 @@ class ClaudeChatProvider { result.sevenDaySonnet = sevenDaySonnet; } - // #35: newer accounts return the per-model weekly windows only as + // fork-issue-35: newer accounts return the per-model weekly windows only as // limits[] entries (kind "weekly_scoped" with a model scope) while the // legacy seven_day_opus/seven_day_sonnet fields stay null. Top-level // fields win when both are present. @@ -3926,7 +3926,7 @@ class ClaudeChatProvider { } // Pushes a (possibly cached) usage-limits snapshot to the webview, throttled to at - // most one real fetch every 5 minutes (#35). Gated on subscription type: API and + // most one real fetch every 5 minutes (fork-issue-35). Gated on subscription type: API and // OpenCredits users have no session/weekly limits to show. private async _maybeSendUsageLimits(): Promise { if (!this._subscriptionType) { @@ -4219,10 +4219,10 @@ class ClaudeChatProvider { } } - // #38 turn diff: walks _currentConversation backwards from messageIndex (inclusive) + // fork-issue-38 turn diff: walks _currentConversation backwards from messageIndex (inclusive) // to the nearest showRestoreOption entry, which is the checkpoint commit made right // before this turn's user message (_createBackupCommit runs before every turn). Works - // both live and after a history reload -- unlike _commits (#50), _currentConversation + // both live and after a history reload -- unlike _commits (fork-issue-50), _currentConversation // is exactly what gets persisted/reloaded, so the index lines up either way. private _findTurnBaselineSha(messageIndex: number): string | undefined { const start = Math.min(messageIndex, this._currentConversation.length - 1); @@ -4260,7 +4260,7 @@ class ClaudeChatProvider { } } - // opus review FIX 2: distinguishes a genuinely new file (nothing existed at the + // review FIX 2: distinguishes a genuinely new file (nothing existed at the // checkpoint yet) from a file that's simply gitignored in the shadow backup repo // (_createBackupCommit's `add -A` silently skips ignored paths) -- both produce the // identical `does not exist in ` from `git show`, but only the first should @@ -4290,9 +4290,9 @@ class ClaudeChatProvider { // returns it as a UTF-8 string, or throws when there's nothing sane to show. Shared // by _openTurnDiff (manual/auto "open diff", already knows workspaceFolder/sha from // the live call) and resolveTurnDiffBaselineForProvider (a DiffContentProvider - // cache miss, opus review FIX 3) so both go through the identical git-show + + // cache miss, review FIX 3) so both go through the identical git-show + // classification + guards, and BOM-stripping only has to happen in one place - // (opus review FIX 5). Never returns a silently-wrong baseline -- callers each + // (review FIX 5). Never returns a silently-wrong baseline -- callers each // decide what "failure" means for their UI (fallback toast/permLog vs. a generic // VS Code tab error). private async _resolveTurnDiffBaseline(backupRepoPath: string, workTreePath: string, sha: string, relPath: string): Promise { @@ -4306,7 +4306,7 @@ class ClaudeChatProvider { content = stdout; } catch (error: any) { const stderrText = Buffer.isBuffer(error?.stderr) ? error.stderr.toString('utf8') : String(error?.stderr || error?.message || ''); - // opus review FIX 2: `exists on disk, but not in ` is deliberately NOT + // review FIX 2: `exists on disk, but not in ` is deliberately NOT // treated as "new file" below. Best effort only: whether git emits that // message (vs. plain `does not exist in`) depends on the process cwd seeing // the on-disk file, so e.g. a case-only mismatch (Src/ vs src/) is not @@ -4335,11 +4335,11 @@ class ClaudeChatProvider { } // VS Code strips the BOM from the real file's text model, keep both sides - // consistent (opus review FIX 5). + // consistent (review FIX 5). return content.toString('utf8').replace(/^\uFEFF/, ''); } - // Public seam for DiffContentProvider's injected resolver (opus review FIX 3, + // Public seam for DiffContentProvider's injected resolver (review FIX 3, // wired up in activate()) -- reuses the same backup-repo baseline lookup // _openTurnDiff uses, keyed only by the (sha, relPath) already encoded in a // claude-diff tab's own URI, so a tab restored via "Reopen Closed Editor" or a VS @@ -4353,12 +4353,12 @@ class ClaudeChatProvider { return this._resolveTurnDiffBaseline(this._backupRepoPath, workspaceFolder.uri.fsPath, sha, relPath); } - // #38: opens a real VS Code diff -- the checkpoint from right before this turn + // fork-issue-38: opens a real VS Code diff -- the checkpoint from right before this turn // (left, read-only virtual document served from the shadow backup repo via // DiffContentProvider) against the actual file on disk (right, live/editable, so // later edits in the same turn keep showing up in the same tab). Shared by the // manual "Open Diff" button and the auto-open after a successful tool_result; - // `trigger` picks which of the two _openTurnDiffFallback behaves as (opus review + // `trigger` picks which of the two _openTurnDiffFallback behaves as (review // FIX 1). private async _openTurnDiff(filePath: string, messageIndex: number, trigger: 'manual' | 'auto'): Promise { const resolvedPath = mapWslPathToWindows(filePath); @@ -4376,7 +4376,7 @@ class ClaudeChatProvider { } // toWorkspaceRelativePath also returns undefined when resolvedPath IS the - // workspace root itself (opus review FIX 4) -- a directory has no checkpointed + // workspace root itself (review FIX 4) -- a directory has no checkpointed // blob to diff against, so it's handled the same as "outside the workspace". const relPath = toWorkspaceRelativePath(resolvedPath, workspaceFolder.uri.fsPath); if (relPath === undefined) { diff --git a/src/restore-commit-utils.ts b/src/restore-commit-utils.ts index 81cbfe9..5874597 100644 --- a/src/restore-commit-utils.ts +++ b/src/restore-commit-utils.ts @@ -1,4 +1,4 @@ -// Pure helpers for the #50 checkpoint-restore fix: after a history load switches +// Pure helpers for the fork-issue-50 checkpoint-restore fix: after a history load switches // conversations, extension.ts's in-memory _commits list is cleared (see // _loadConversationHistory) even though the replayed showRestoreOption messages still // show a working Restore button for a checkpoint that still exists in the shadow @@ -24,7 +24,7 @@ export function isValidCommitSha(sha: string): boolean { // Recovers a commit's display info (message/timestamp for the restore toasts) from the // matching showRestoreOption entry replayed into _currentConversation, for a sha that -// _commits no longer knows about (#50: cleared by a history load that switched +// _commits no longer knows about (fork-issue-50: cleared by a history load that switched // conversations). Only called once the caller has independently confirmed sha still // exists in the backup repo -- this never claims a sha exists, only recovers its // metadata, and falls back to a minimal placeholder built from the sha itself when no diff --git a/src/script.ts b/src/script.ts index ab37901..2aa70d5 100644 --- a/src/script.ts +++ b/src/script.ts @@ -81,7 +81,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt let isWindows = false; let attachedImages = []; // Array of { filePath, previewUri } - // #38: request a real VS Code diff (checkpoint-before-turn vs. the live file) for + // fork-issue-38: request a real VS Code diff (checkpoint-before-turn vs. the live file) for // one Edit/MultiEdit/Write message. filePath/messageIndex come from the clicked // button's own dataset (see generateUnifiedDiffHTML/formatMultiEditToolDiff), not // a shared pending-edit slot, so the button keeps working after tool_result and @@ -243,7 +243,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Format raw input with expandable content for long values // Use diff format for Edit, MultiEdit, and Write tools, regular format for others if (data.toolName === 'Edit' || data.toolName === 'MultiEdit' || data.toolName === 'Write') { - // #38: the Open Diff button stays visible after tool_result and after a + // fork-issue-38: the Open Diff button stays visible after tool_result and after a // history reload -- it only needs a valid messageIndex (used to look up // the pre-turn checkpoint on the host side), not the live-only, // optimistic fileContentBefore read. @@ -317,7 +317,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const messagesDiv = document.getElementById('messages'); const shouldScroll = shouldAutoScroll(messagesDiv); - // #38: the Open Diff button on the request no longer gets hidden when its + // fork-issue-38: the Open Diff button on the request no longer gets hidden when its // result arrives -- it stays available (and auto-open, if enabled, has // already opened/updated the same turn diff by the time this runs). @@ -1021,13 +1021,12 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } // Approximate context-window size per model, used to turn currentContextTokens - // into a percentage for the status bar (#27). Best-effort approximation, not the + // into a percentage for the status bar (fork-issue-27). Best-effort approximation, not the // model's authoritative limit — router models use context_length from the - // recommended-models catalog. Native fable/opus/sonnet are the 1M-token variants - // per user decision (this setup runs on those); 'default' and unknown models - // fall back to a conservative 200K (underestimating only warns early). + // recommended-models catalog. 'default' and unknown models fall back to a + // conservative 200K (underestimating only warns early). function getContextWindow(model) { - const nativeWindows = { fable: 1000000, opus: 1000000, sonnet: 1000000, 'default': 200000 }; + const nativeWindows = { opus: 200000, sonnet: 200000, 'default': 200000 }; if (nativeWindows[model]) { return nativeWindows[model]; } @@ -1036,7 +1035,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } // Builds the "Ctx 12,345 / ~200K (62%)" status-bar fragment, with a warning/ - // critical class once usage crosses 80%/95% (#27). Empty string when there's no + // critical class once usage crosses 80%/95% (fork-issue-27). Empty string when there's no // context reading yet, so the status line looks exactly like before in that case. function getContextIndicatorHtml() { if (!currentContextTokens || currentContextTokens <= 0) { @@ -1049,8 +1048,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt return \` • Ctx \${currentContextTokens.toLocaleString()} / ~\${winStr} (\${pct}%)\`; } - // Builds the "5h 42% · Week 18% · Opus 30%" status-bar fragment (#35), same - // structure/escaping as the #27 Ctx indicator above. Opus/Sonnet are the + // Builds the "5h 42% · Week 18% · Opus 30%" status-bar fragment (fork-issue-35), same + // structure/escaping as the fork-issue-27 Ctx indicator above. Opus/Sonnet are the // per-model weekly buckets (seven_day_opus/seven_day_sonnet); each renders // only when the account's usage data actually includes it. Empty string when // there's no usage data yet. @@ -1107,7 +1106,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // OpenCredits users: don't show tokens, just elapsed time statusText = \`Processing\${elapsedStr ? \` • \${elapsedStr}\` : ''}\`; } else { - // Regular users: show context usage and elapsed time (#27 — the + // Regular users: show context usage and elapsed time (fork-issue-27 — the // context indicator replaced the old cumulative token sum here) statusText = \`Processing\${getContextIndicatorHtml()}\${getUsageIndicatorHtml()}\${elapsedStr ? \` • \${elapsedStr}\` : ''}\`; } @@ -1142,7 +1141,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const requestStr = requestCount > 0 ? \`\${requestCount} requests\` : ''; statusText = \`Ready\${requestStr ? \` • \${requestStr}\` : ''} • \${usageStr}\`; } else { - // Regular users: show context usage, requests, and usage (#27 — the + // Regular users: show context usage, requests, and usage (fork-issue-27 — the // context indicator replaced the old cumulative token sum here) const requestStr = requestCount > 0 ? \`\${requestCount} requests\` : ''; statusText = \`Ready\${getContextIndicatorHtml()}\${getUsageIndicatorHtml()}\${requestStr ? \` • \${requestStr}\` : ''} • \${usageStr}\`; @@ -3758,7 +3757,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt break; case 'usageLimits': - // Store session-usage / weekly-limit snapshot (#35) and refresh the status bar + // Store session-usage / weekly-limit snapshot (fork-issue-35) and refresh the status bar latestUsage = message.data || null; updateStatusWithTotals(); break; @@ -4896,7 +4895,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const yoloMode = document.getElementById('yolo-mode').checked; const executablePath = document.getElementById('executable-path').value; const useRouter = document.getElementById('use-router')?.checked || false; - // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write + // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write const diffAutoOpen = document.getElementById('diff-auto-open').checked; // Collect environment variables from key-value UI @@ -5195,7 +5194,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt }); } else if (message.type === 'settingsData') { // Update UI with current settings - // #38: auto-open a turn diff after a successful Edit/MultiEdit/Write + // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write document.getElementById('diff-auto-open').checked = message.data['diff.autoOpen'] !== false; const thinkingIntensity = message.data['thinking.intensity'] || 'think'; const intensityValues = ['think', 'think-hard', 'think-harder', 'ultrathink']; diff --git a/src/test/diff-utils.test.ts b/src/test/diff-utils.test.ts index 0b1a0fe..d857714 100644 --- a/src/test/diff-utils.test.ts +++ b/src/test/diff-utils.test.ts @@ -1,8 +1,8 @@ -// Unit tests for the #38 turn-diff helpers (WSL path mapping, workspace-relative +// Unit tests for the fork-issue-38 turn-diff helpers (WSL path mapping, workspace-relative // path resolution, binary detection, baseline URI construction). All pure (no // vscode, no network, no filesystem access), so these run under plain mocha against -// the compiled out/ output -- same pattern as the shell-utils/auto-model-switch unit -// tests. Run with `npm run test:diff-utils`. +// the compiled out/ output -- same pattern as the model-updater unit tests. Run with +// `npm run test:diff-utils`. import * as assert from 'assert'; import { @@ -18,7 +18,7 @@ import { suite('diff-utils: mapWslPathToWindows', () => { test('maps /mnt/c/... to C:\\...', () => { - assert.strictEqual(mapWslPathToWindows('/mnt/c/Users/Roman/foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + assert.strictEqual(mapWslPathToWindows('/mnt/c/Users/example/foo.ts'), 'C:\\Users\\example\\foo.ts'); }); test('maps other drive letters too (e.g. /mnt/d)', () => { @@ -26,15 +26,15 @@ suite('diff-utils: mapWslPathToWindows', () => { }); test('is case-insensitive on the drive letter and normalizes it to uppercase', () => { - assert.strictEqual(mapWslPathToWindows('/mnt/C/Users/Roman/foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + assert.strictEqual(mapWslPathToWindows('/mnt/C/Users/example/foo.ts'), 'C:\\Users\\example\\foo.ts'); }); test('leaves an already-Windows path unchanged', () => { - assert.strictEqual(mapWslPathToWindows('C:\\Users\\Roman\\foo.ts'), 'C:\\Users\\Roman\\foo.ts'); + assert.strictEqual(mapWslPathToWindows('C:\\Users\\example\\foo.ts'), 'C:\\Users\\example\\foo.ts'); }); test('leaves a non-/mnt Linux path unchanged (not a WSL-mapped drive)', () => { - assert.strictEqual(mapWslPathToWindows('/home/roman/foo.ts'), '/home/roman/foo.ts'); + assert.strictEqual(mapWslPathToWindows('/home/user/foo.ts'), '/home/user/foo.ts'); }); }); diff --git a/src/test/restore-commit-utils.test.ts b/src/test/restore-commit-utils.test.ts index aac5fe6..d47d0a6 100644 --- a/src/test/restore-commit-utils.test.ts +++ b/src/test/restore-commit-utils.test.ts @@ -1,7 +1,7 @@ -// Unit tests for the #50 checkpoint-restore fix (isValidCommitSha, +// Unit tests for the fork-issue-50 checkpoint-restore fix (isValidCommitSha, // findRehydratedCommitInfo). Pure (no vscode, no network, no filesystem access), so // these run under plain mocha against the compiled out/ output -- same pattern as -// diff-utils/shell-utils/auto-model-switch. The actual "does this commit still exist" +// diff-utils/model-updater. The actual "does this commit still exist" // check (git cat-file -e against the shadow backup repo) stays in extension.ts, // untested here, same as diff-utils' git-show baseline read. Run with // `npm run test:restore-commit-utils`. From da55e884251f32d43edfedac8c9d43c4dedcae27 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 20:52:00 +0200 Subject: [PATCH 10/26] fix: wire currentContextTokens from extension host to webview status line The fork-issue-27 context indicator (getContextIndicatorHtml) was reading a currentContextTokens variable that was never assigned anywhere, because the extension.ts half of the plumbing (the _currentContextTokens field, computing it per turn, and sending it on the updateTokens postMessage) had been dropped. The function silently always returned '', which had quietly replaced the previous "N tokens" status-line display with nothing for non-subscription users. Restores the field, the per-turn ctx calculation (input + cache read + cache creation tokens), the three reset points (compact boundary, new session, loaded conversation), and the webview-side assignment/resets in script.ts. Also adds the missing .ctx-warn/.ctx-crit CSS rules the 80%/95% warning classes rely on, which existed nowhere in ui-styles.ts. --- src/extension.ts | 17 ++++++++++++++++- src/script.ts | 5 ++++- src/ui-styles.ts | 9 +++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 3cba698..6c6d717 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -246,6 +246,9 @@ class ClaudeChatProvider { private _totalCost: number = 0; private _totalTokensInput: number = 0; private _totalTokensOutput: number = 0; + // Non-cumulative holder for the most recent turn's context usage (input + + // cache read + cache creation tokens), unlike the cumulative counters above (fork-issue-27). + private _currentContextTokens: number = 0; private _requestCount: number = 0; private _subscriptionType: string | undefined; // 'pro', 'max', or undefined for API users // Session-usage / weekly-limit snapshot from the undocumented oauth/usage endpoint @@ -1457,6 +1460,7 @@ class ClaudeChatProvider { // Reset tokens since the conversation is now summarized this._totalTokensInput = 0; this._totalTokensOutput = 0; + this._currentContextTokens = 0; this._sendAndSaveMessage({ type: 'compactBoundary', @@ -1475,6 +1479,14 @@ class ClaudeChatProvider { this._totalTokensInput += jsonData.message.usage.input_tokens || 0; this._totalTokensOutput += jsonData.message.usage.output_tokens || 0; + // Non-cumulative context estimate for the current turn: input + cache + // read + cache creation tokens are what actually occupies the model's + // context window, unlike the cumulative counters above (fork-issue-27). + const ctx = (jsonData.message.usage.input_tokens || 0) + + (jsonData.message.usage.cache_read_input_tokens || 0) + + (jsonData.message.usage.cache_creation_input_tokens || 0); + this._currentContextTokens = ctx; + // Send real-time token update to webview this._sendAndSaveMessage({ type: 'updateTokens', @@ -1484,7 +1496,8 @@ class ClaudeChatProvider { currentInputTokens: jsonData.message.usage.input_tokens || 0, currentOutputTokens: jsonData.message.usage.output_tokens || 0, cacheCreationTokens: jsonData.message.usage.cache_creation_input_tokens || 0, - cacheReadTokens: jsonData.message.usage.cache_read_input_tokens || 0 + cacheReadTokens: jsonData.message.usage.cache_read_input_tokens || 0, + currentContextTokens: ctx } }); } @@ -1801,6 +1814,7 @@ class ClaudeChatProvider { this._totalCost = 0; this._totalTokensInput = 0; this._totalTokensOutput = 0; + this._currentContextTokens = 0; this._requestCount = 0; // Notify webview to clear all messages and reset session @@ -3481,6 +3495,7 @@ class ClaudeChatProvider { this._totalCost = conversationData.totalCost || 0; this._totalTokensInput = conversationData.totalTokens?.input || 0; this._totalTokensOutput = conversationData.totalTokens?.output || 0; + this._currentContextTokens = 0; // Clear UI messages first, then send all messages to recreate the conversation setTimeout(() => { diff --git a/src/script.ts b/src/script.ts index 2aa70d5..e267bb6 100644 --- a/src/script.ts +++ b/src/script.ts @@ -3711,7 +3711,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update token totals in real-time totalTokensInput = message.data.totalTokensInput || 0; totalTokensOutput = message.data.totalTokensOutput || 0; - + currentContextTokens = message.data.currentContextTokens || currentContextTokens; + // Update status bar immediately updateStatusWithTotals(); @@ -3789,6 +3790,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt totalCost = 0; totalTokensInput = 0; totalTokensOutput = 0; + currentContextTokens = 0; requestCount = 0; updateStatusWithTotals(); break; @@ -3803,6 +3805,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Reset token counts since conversation was compacted totalTokensInput = 0; totalTokensOutput = 0; + currentContextTokens = 0; updateStatusWithTotals(); const preTokens = message.data.preTokens ? message.data.preTokens.toLocaleString() : 'unknown'; diff --git a/src/ui-styles.ts b/src/ui-styles.ts index 76bd766..d05ae4e 100644 --- a/src/ui-styles.ts +++ b/src/ui-styles.ts @@ -3682,6 +3682,15 @@ const styles = ` transform: translateY(0); } + .status-text .ctx-warn { + color: var(--vscode-editorWarning-foreground); + } + + .status-text .ctx-crit { + color: var(--vscode-editorError-foreground); + font-weight: 600; + } + .status-text .usage-icon { width: 12px; height: 12px; From 4dc7100ab8f1cba8fb1f4f1fc80a49d1f505488c Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 20:52:21 +0200 Subject: [PATCH 11/26] docs: scrub remaining internal-process wording from turn-diff comments A first pass missed several inline code-review shorthand notes sprinkled through the fork-issue-38 turn-diff code. Neutralized all 15 spots, keeping the substantive explanation in each comment. --- src/diff-utils.ts | 4 ++-- src/extension.ts | 35 +++++++++++++++++------------------ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/diff-utils.ts b/src/diff-utils.ts index 81c9333..b23dfd5 100644 --- a/src/diff-utils.ts +++ b/src/diff-utils.ts @@ -24,7 +24,7 @@ export function mapWslPathToWindows(filePath: string): string { // case-insensitive on Windows, where the filesystem is case-insensitive but tool // input paths and the workspace folder path aren't guaranteed to agree on casing. // Returns undefined when filePath isn't inside workspaceRoot ("not mappable"), which -// also covers filePath being the workspace root itself (review FIX 4: a +// also covers filePath being the workspace root itself (a // directory has no checkpointed blob to diff against, so treat it the same as // "outside the workspace" instead of handing callers a '' relPath). export function toWorkspaceRelativePath(filePath: string, workspaceRoot: string): string | undefined { @@ -88,7 +88,7 @@ export function buildTurnDiffUriParts(sha: string, relPath: string): TurnDiffUri }; } -// Inverse of buildTurnDiffUriParts (review FIX 3): recovers (sha, relPath) from +// Inverse of buildTurnDiffUriParts: recovers (sha, relPath) from // a claude-diff URI's own (path, query), so DiffContentProvider can resolve a cache // miss -- a tab restored via "Reopen Closed Editor" or a VS Code restart, after the // in-memory diffContentStore is gone -- without needing any other state. vscode.Uri diff --git a/src/extension.ts b/src/extension.ts index 6c6d717..294623a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -42,7 +42,7 @@ const PERM_LOG_FILE = path.join(os.tmpdir(), 'claude-code-chat-perm.log'); // Storage for diff content (used by DiffContentProvider). Keyed by turnDiffCacheKey() // (path+query) so two turns diffing the same relPath under different checkpoint SHAs -// don't collide on the same entry. Bounded (review FIX 3): entries used to be +// don't collide on the same entry. Bounded: entries used to be // removed by an onDidCloseTextDocument listener, which neither fired for every tab // lifecycle (e.g. vscode.diff throwing after the entry was already stored) nor could // ever help resolve a cache miss -- a tab restored via "Reopen Closed Editor" or a VS @@ -67,7 +67,7 @@ function cacheTurnDiffContent(key: string, content: string): void { // the pre-turn checkpoint content as the left/baseline side of vscode.diff). Content // is normally already cached (written by _openTurnDiff right before vscode.diff is // invoked), but a cache miss -- e.g. a claude-diff tab restored via "Reopen Closed -// Editor" or after a VS Code restart, see review FIX 3 -- is resolved on demand +// Editor" or after a VS Code restart -- is resolved on demand // through the injected resolver, using only the (sha, relPath) already baked into the // URI itself (see parseTurnDiffUriParts), so the provider needs no other state. class DiffContentProvider implements vscode.TextDocumentContentProvider { @@ -128,7 +128,7 @@ export function activate(context: vscode.ExtensionContext) { vscode.window.registerWebviewViewProvider('claude-code-chat.chat', webviewProvider); // Register custom content provider for read-only diff views. Wired to the primary - // provider's baseline resolver (review FIX 3) -- extra panels from "New Claude + // provider's baseline resolver -- extra panels from "New Claude // Chat (Separate)" (fork-issue-24) share the same extension context, so they resolve to the // same backup repo anyway; a claude-diff tab has no panel of its own to route to. const diffProvider = new DiffContentProvider((sha, relPath) => provider.resolveTurnDiffBaselineForProvider(sha, relPath)); @@ -1674,7 +1674,7 @@ class ClaudeChatProvider { // once per file per turn (see _autoOpenedDiffFilesThisTurn reset in // _sendMessageToClaude). Manual "Open Diff" clicks go through the same // _openTurnDiff but aren't gated by the setting or this dedup set. - // trigger: 'auto' (review FIX 1) -- Claude sessions routinely edit + // trigger: 'auto' -- Claude sessions routinely edit // files outside the workspace (scratchpad, ~/.claude memory, etc.), so // _openTurnDiff failing here is the ordinary case, not something to // interrupt the user with a toast/focus-stealing showTextDocument for. @@ -4253,8 +4253,8 @@ class ClaudeChatProvider { // Shared failure path for every way _openTurnDiff can come up short (no checkpoint, // git error, file outside the workspace/not WSL-mappable, too large/binary baseline): // never fail silently for a real user click -- tell them why there's no diff and - // open the real file instead so a click is never a dead end. `trigger` (opus - // review FIX 1) tells 'manual' (webview "Open Diff" button, a deliberate user + // open the real file instead so a click is never a dead end. `trigger` + // tells 'manual' (webview "Open Diff" button, a deliberate user // action -- toast + focus is fine) apart from 'auto' (post tool_result auto-open, // see the Edit/MultiEdit/Write handler above): Claude sessions routinely edit files // outside the workspace (scratchpad, ~/.claude memory, etc.), so failing here is @@ -4275,7 +4275,7 @@ class ClaudeChatProvider { } } - // review FIX 2: distinguishes a genuinely new file (nothing existed at the + // Distinguishes a genuinely new file (nothing existed at the // checkpoint yet) from a file that's simply gitignored in the shadow backup repo // (_createBackupCommit's `add -A` silently skips ignored paths) -- both produce the // identical `does not exist in ` from `git show`, but only the first should @@ -4288,7 +4288,7 @@ class ClaudeChatProvider { // cwd pinned to the work tree: git resolves the relative path against the // process cwd's prefix inside the work tree, so an unpinned cwd would make // anchored .gitignore entries (like /out/) match or miss depending on where - // the extension host happens to run (opus delta-review). + // the extension host happens to run. await execFile('git', ['--git-dir', backupRepoPath, '--work-tree', workTreePath, 'check-ignore', '-q', '--', relPath], { cwd: workTreePath }); return true; } catch (error: any) { @@ -4305,9 +4305,9 @@ class ClaudeChatProvider { // returns it as a UTF-8 string, or throws when there's nothing sane to show. Shared // by _openTurnDiff (manual/auto "open diff", already knows workspaceFolder/sha from // the live call) and resolveTurnDiffBaselineForProvider (a DiffContentProvider - // cache miss, review FIX 3) so both go through the identical git-show + - // classification + guards, and BOM-stripping only has to happen in one place - // (review FIX 5). Never returns a silently-wrong baseline -- callers each + // cache miss) so both go through the identical git-show + + // classification + guards, and BOM-stripping only has to happen in one place. + // Never returns a silently-wrong baseline -- callers each // decide what "failure" means for their UI (fallback toast/permLog vs. a generic // VS Code tab error). private async _resolveTurnDiffBaseline(backupRepoPath: string, workTreePath: string, sha: string, relPath: string): Promise { @@ -4321,7 +4321,7 @@ class ClaudeChatProvider { content = stdout; } catch (error: any) { const stderrText = Buffer.isBuffer(error?.stderr) ? error.stderr.toString('utf8') : String(error?.stderr || error?.message || ''); - // review FIX 2: `exists on disk, but not in ` is deliberately NOT + // `exists on disk, but not in ` is deliberately NOT // treated as "new file" below. Best effort only: whether git emits that // message (vs. plain `does not exist in`) depends on the process cwd seeing // the on-disk file, so e.g. a case-only mismatch (Src/ vs src/) is not @@ -4350,12 +4350,12 @@ class ClaudeChatProvider { } // VS Code strips the BOM from the real file's text model, keep both sides - // consistent (review FIX 5). + // consistent. return content.toString('utf8').replace(/^\uFEFF/, ''); } - // Public seam for DiffContentProvider's injected resolver (review FIX 3, - // wired up in activate()) -- reuses the same backup-repo baseline lookup + // Public seam for DiffContentProvider's injected resolver (wired up in + // activate()) -- reuses the same backup-repo baseline lookup // _openTurnDiff uses, keyed only by the (sha, relPath) already encoded in a // claude-diff tab's own URI, so a tab restored via "Reopen Closed Editor" or a VS // Code restart can resolve itself without any per-turn state. Errors are left for @@ -4373,8 +4373,7 @@ class ClaudeChatProvider { // DiffContentProvider) against the actual file on disk (right, live/editable, so // later edits in the same turn keep showing up in the same tab). Shared by the // manual "Open Diff" button and the auto-open after a successful tool_result; - // `trigger` picks which of the two _openTurnDiffFallback behaves as (review - // FIX 1). + // `trigger` picks which of the two _openTurnDiffFallback behaves as. private async _openTurnDiff(filePath: string, messageIndex: number, trigger: 'manual' | 'auto'): Promise { const resolvedPath = mapWslPathToWindows(filePath); @@ -4391,7 +4390,7 @@ class ClaudeChatProvider { } // toWorkspaceRelativePath also returns undefined when resolvedPath IS the - // workspace root itself (review FIX 4) -- a directory has no checkpointed + // workspace root itself -- a directory has no checkpointed // blob to diff against, so it's handled the same as "outside the workspace". const relPath = toWorkspaceRelativePath(resolvedPath, workspaceFolder.uri.fsPath); if (relPath === undefined) { From d686a45340f81266c2b9254331731969617e6b5c Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 21:28:20 +0200 Subject: [PATCH 12/26] fix: log only the basename in the turn-diff auto-skip perm-log line _permLog (fork-issue-15) writes unrotated plaintext to os.tmpdir(), so the auto-skip diagnostic for _openTurnDiffFallback was leaking the full, potentially sensitive file path on every skip. path.basename(filePath) is enough to diagnose which file was involved without exposing the path. --- src/extension.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 294623a..cf1c4cf 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -4263,8 +4263,9 @@ class ClaudeChatProvider { private async _openTurnDiffFallback(filePath: string, trigger: 'manual' | 'auto', reason: string): Promise { if (trigger === 'auto') { // First line only: git error messages can be multi-line and would break the - // one-line-per-entry perm-log format. - this._permLog(`[turndiff] auto skip reason=${reason.split('\n')[0]} file=${filePath}`); + // one-line-per-entry perm-log format. Basename only -- the perm-log file is + // unrotated plaintext, so the full path isn't worth leaking for a diagnostic line. + this._permLog(`[turndiff] auto skip reason=${reason.split('\n')[0]} file=${path.basename(filePath)}`); return; } vscode.window.showInformationMessage(`Claude Code Chat: ${reason}; showing the file instead.`); From e9e02a0b3b5ae01dff26741ffd7492d496d28ae2 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 21:56:56 +0200 Subject: [PATCH 13/26] docs: fix stale fork-issue-24 comment, drop two stray blank lines The diff-content-provider comment justified skipping per-panel routing by pointing at a "New Claude Chat (Separate)" (fork-issue-24) feature that doesn't exist in this branch (no such command in package.json, no second panel type) -- reworded to describe what's actually true: a single shared ClaudeChatProvider instance backs both the panel command and the sidebar webview, so there's only ever one provider to resolve against regardless of who opened the tab. Also removes two blank lines that an earlier commit in this branch introduced incidentally (one in extension.ts before the rate_limit_event usage-limits refresh, one in script.ts before renderEnvVariables) and that don't match the surrounding code's spacing. --- src/extension.ts | 9 ++++----- src/script.ts | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index cf1c4cf..d53d7d0 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -127,10 +127,10 @@ export function activate(context: vscode.ExtensionContext) { const webviewProvider = new ClaudeChatWebviewProvider(context.extensionUri, provider); vscode.window.registerWebviewViewProvider('claude-code-chat.chat', webviewProvider); - // Register custom content provider for read-only diff views. Wired to the primary - // provider's baseline resolver -- extra panels from "New Claude - // Chat (Separate)" (fork-issue-24) share the same extension context, so they resolve to the - // same backup repo anyway; a claude-diff tab has no panel of its own to route to. + // Register custom content provider for read-only diff views. Wired to the single + // shared ClaudeChatProvider instance's baseline resolver -- both the panel command + // and the sidebar webview use this same instance, so a claude-diff tab always + // resolves to the same backup repo regardless of which one opened it. const diffProvider = new DiffContentProvider((sha, relPath) => provider.resolveTurnDiffBaselineForProvider(sha, relPath)); context.subscriptions.push(vscode.workspace.registerTextDocumentContentProvider('claude-diff', diffProvider)); @@ -1782,7 +1782,6 @@ class ClaudeChatProvider { if (!rateLimitType || rateLimitType === 'five_hour') { this._lastRateLimitResetsAt = jsonData.rate_limit_info?.resetsAt; } - void this._maybeSendUsageLimits(); break; } diff --git a/src/script.ts b/src/script.ts index e267bb6..db0293c 100644 --- a/src/script.ts +++ b/src/script.ts @@ -5233,7 +5233,6 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update Customize Claude Command settings document.getElementById('executable-path').value = message.data['executable.path'] || ''; - renderEnvVariables(message.data['environment.variables'] || {}); // Detect OpenCredits and envs disabled state From 9c03a2e5015e04eadf434eb167748354da9bd14b Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Wed, 29 Jul 2026 05:29:35 +0200 Subject: [PATCH 14/26] fix: wire dead KNOWN_ENDPOINT_MARKERS constant, redact reason= path leak, fix stale cat-file comment Review-round follow-up: KNOWN_ENDPOINT_MARKERS was declared but never read -- _isOpenCredits() kept checking the same two literals by hand. Now _isOpenCredits() actually uses the constant, so there's a single place to add a future endpoint marker. _openTurnDiffFallback's auto-skip perm-log line still leaked the full backup-repo/file path through its reason= field even after the earlier basename-only fix for file=: _resolveTurnDiffBaseline wraps raw execFile failures whose message starts with the full command line. reason now goes through the same basename collapsing before logging. Also corrects a comment that misattributed a "plain cat-file, exit 1" contrast to _isPathIgnoredInBackupRepo (which uses `git check-ignore -q`, not cat-file) instead of the actual unpeeled `git cat-file -e` case it meant to describe. Co-Authored-By: Claude Sonnet 5 --- src/extension.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index d53d7d0..5e91d73 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -404,7 +404,7 @@ class ClaudeChatProvider { } const envVars = config.get>('environment.variables', {}); const baseUrl = envVars['ANTHROPIC_BASE_URL'] || ''; - return baseUrl.includes('opencredits.ai') || baseUrl.includes('localhost:8787'); + return KNOWN_ENDPOINT_MARKERS.some(marker => baseUrl.includes(marker)); } private async _setEnvsDisabled(disabled: boolean): Promise { @@ -2007,10 +2007,11 @@ class ClaudeChatProvider { } catch (error: any) { // With the ^{commit} peel, git reports both "sha missing" and "sha // not a commit" as exit 128 + "fatal: Not a valid object name" (not - // exit 1 like _isPathIgnoredInBackupRepo's plain cat-file), so - // classify on stderr like _resolveTurnDiffBaseline does: that text - // is the silent, expected miss; anything else (ENOENT, broken - // backup repo) is real infrastructure failure worth a log line. + // exit 1, which a plain, unpeeled `git cat-file -e ` would report + // for a simply-missing object), so classify on stderr like + // _resolveTurnDiffBaseline does: that text is the silent, expected + // miss; anything else (ENOENT, broken backup repo) is real + // infrastructure failure worth a log line. const stderrText = String(error?.stderr || ''); if (!/Not a valid object name/i.test(stderrText)) { console.error('Failed to check commit existence in backup repo:', error.message); @@ -4264,7 +4265,12 @@ class ClaudeChatProvider { // First line only: git error messages can be multi-line and would break the // one-line-per-entry perm-log format. Basename only -- the perm-log file is // unrotated plaintext, so the full path isn't worth leaking for a diagnostic line. - this._permLog(`[turndiff] auto skip reason=${reason.split('\n')[0]} file=${path.basename(filePath)}`); + // reason needs the same treatment: _resolveTurnDiffBaseline wraps raw execFile + // failures, whose message starts with the full command line (absolute + // backup-repo path, workspace-relative file path included) -- collapse any + // path-looking token down to its basename before it hits the log. + const redactedReason = reason.split('\n')[0].replace(/[^\s"']*[\\/][^\s"']*/g, (token) => path.basename(token)); + this._permLog(`[turndiff] auto skip reason=${redactedReason} file=${path.basename(filePath)}`); return; } vscode.window.showInformationMessage(`Claude Code Chat: ${reason}; showing the file instead.`); From 9aa037748ab5b61597d94e316a429cc2afdabb60 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 09:26:58 +0200 Subject: [PATCH 15/26] feat: offer a shortcut to raise CLAUDE_CODE_MAX_OUTPUT_TOKENS (fork-issue-42) New setting claudeCodeChat.advanced.maxOutputTokens (integer, 0 = CLI default) feeds the env variable on both the native and the WSL spawn path. When the CLI reports 'API Error: ... output token maximum' - it arrives as assistant text, verified against the live CLI - the chat shows a hint with a button that opens the settings UI filtered to the new setting. Co-Authored-By: Claude Fable 5 --- package.json | 6 ++++++ src/extension.ts | 14 +++++++++++++- src/script.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 74d9676..6e7d5d8 100644 --- a/package.json +++ b/package.json @@ -194,6 +194,12 @@ "default": "", "description": "Custom path to the Claude Code executable. Leave empty to use the default 'claude' command." }, + "claudeCodeChat.advanced.maxOutputTokens": { + "type": "number", + "default": 0, + "minimum": 0, + "description": "Maximum number of tokens Claude may generate in a single response (sets the CLAUDE_CODE_MAX_OUTPUT_TOKENS environment variable for the CLI). Increase this if you hit a \"response exceeded the output token maximum\" error. 0 = use the CLI default." + }, "claudeCodeChat.environment.variables": { "type": "object", "default": {}, diff --git a/src/extension.ts b/src/extension.ts index 5e91d73..bc5ce23 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -784,6 +784,11 @@ class ClaudeChatProvider { case 'enableYoloMode': this._enableYoloMode(); return; + case 'openMaxOutputTokensSettings': + // #42: deep-link into the native Settings UI, filtered on our setting. + // No value is set automatically - the user picks the limit themselves. + vscode.commands.executeCommand('workbench.action.openSettings', 'claudeCodeChat.advanced.maxOutputTokens'); + return; case 'saveInputText': this._saveInputText(message.text); return; @@ -1096,6 +1101,7 @@ class ClaudeChatProvider { const customExecutablePath = config.get('executable.path', ''); const envsDisabled = config.get('environment.disabled', false); const customEnvVars = envsDisabled ? {} : config.get>('environment.variables', {}); + const maxOutputTokens = config.get('advanced.maxOutputTokens', 0); // Check if using OpenCredits (base URL contains opencredits.ai) const isOpenCredits = this._isOpenCredits(); @@ -1116,7 +1122,10 @@ class ClaudeChatProvider { FORCE_COLOR: '0', NO_COLOR: '1', ...customEnvVars, // Apply custom environment variables (ANTHROPIC_AUTH_TOKEN, ANTHROPIC_BASE_URL, etc.) - CLAUDE_CODE_ENTRYPOINT: 'claude-vscode' + CLAUDE_CODE_ENTRYPOINT: 'claude-vscode', + // #42: raise the CLI's response size cap when configured, to work around + // "response exceeded the output token maximum" errors (upstream #150) + ...(Math.floor(maxOutputTokens) > 0 ? { CLAUDE_CODE_MAX_OUTPUT_TOKENS: String(Math.floor(maxOutputTokens)) } : {}) }; // OpenCredits: clear Anthropic-specific vars so Claude CLI uses env vars directly @@ -1150,6 +1159,9 @@ class ClaudeChatProvider { wslEnvOverrides['DISABLE_COST_WARNINGS'] = 'true'; } wslEnvOverrides['CLAUDE_CODE_ENTRYPOINT'] = 'claude-vscode'; + if (Math.floor(maxOutputTokens) > 0) { + wslEnvOverrides['CLAUDE_CODE_MAX_OUTPUT_TOKENS'] = String(Math.floor(maxOutputTokens)); + } const envExports = Object.entries(wslEnvOverrides) .map(([k, v]) => `export ${k}="${v.replace(/"/g, '\\"')}"`) .join(' && '); diff --git a/src/script.ts b/src/script.ts index db0293c..22203e9 100644 --- a/src/script.ts +++ b/src/script.ts @@ -188,6 +188,20 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt messageDiv.appendChild(yoloSuggestion); } + // Check if this is an output token limit error and offer a shortcut to + // raise CLAUDE_CODE_MAX_OUTPUT_TOKENS via the setting (#42, upstream #150) + if ((type === 'error' || type === 'claude') && isOutputTokenLimitError(content)) { + const tokenLimitSuggestion = document.createElement('div'); + tokenLimitSuggestion.className = 'yolo-suggestion'; + tokenLimitSuggestion.innerHTML = \` +
+ 💡 Claude's response exceeded the output token limit. You can raise the limit in settings. +
+ + \`; + messageDiv.appendChild(tokenLimitSuggestion); + } + messagesDiv.appendChild(messageDiv); moveProcessingIndicatorToLast(); scrollToBottomIfNeeded(messagesDiv, shouldScroll); @@ -1434,6 +1448,13 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt ); } + function isOutputTokenLimitError(content) { + // Require the "API Error:" prefix so this only fires on the actual CLI + // error text, not on ordinary conversation that happens to mention the + // output token maximum (e.g. the user asking about this very feature). + return /API Error:.*output token maximum/i.test(content); + } + function enableYoloMode() { sendStats('YOLO mode enabled'); @@ -1453,6 +1474,14 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } } + function openMaxOutputTokensSettings() { + sendStats('Output token limit settings opened'); + + vscode.postMessage({ + type: 'openMaxOutputTokensSettings' + }); + } + function hideMCPModal() { document.getElementById('mcpModal').style.display = 'none'; hideAddServerForm(); From 619ff0755ea806d81bb1352ec2e711959875ed90 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 09:23:33 +0200 Subject: [PATCH 16/26] feat: configurable chat font family and size (fork-issue-44) Two new settings under claudeCodeChat.ui apply via CSS variables set with style.setProperty (no markup interpolation). The .messages and .input-field font-size rules, and the per-bubble font-family rules (user, tool-result, thinking), respect the variables so the custom font stays consistent, and the input height re-measures when the size changes. Co-Authored-By: Claude Fable 5 --- package.json | 12 ++++++++++++ src/extension.ts | 2 ++ src/script.ts | 20 ++++++++++++++++++++ src/ui-styles.ts | 13 +++++++------ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 6e7d5d8..6a768a0 100644 --- a/package.json +++ b/package.json @@ -219,6 +219,18 @@ "type": "boolean", "default": true, "description": "Automatically open a VS Code diff view after Claude successfully edits or creates a file, comparing it against the checkpoint from before the current turn." + }, + "claudeCodeChat.ui.fontFamily": { + "type": "string", + "default": "", + "description": "Custom font family for the chat message area and input field. Leave empty to use the editor's default font." + }, + "claudeCodeChat.ui.fontSize": { + "type": "number", + "default": 0, + "minimum": 0, + "maximum": 72, + "description": "Custom font size (px, 6-72) for the chat message area and input field. Leave at 0 to use the editor's default font size." } } } diff --git a/src/extension.ts b/src/extension.ts index bc5ce23..fab043c 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3620,6 +3620,8 @@ class ClaudeChatProvider { 'environment.variables': config.get>('environment.variables', {}), 'environment.disabled': config.get('environment.disabled', false), 'diff.autoOpen': config.get('diff.autoOpen', true), + 'ui.fontFamily': config.get('ui.fontFamily', ''), + 'ui.fontSize': config.get('ui.fontSize', 0), 'isOpenCredits': this._isOpenCredits() }; diff --git a/src/script.ts b/src/script.ts index 22203e9..cd2818a 100644 --- a/src/script.ts +++ b/src/script.ts @@ -5228,6 +5228,26 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update UI with current settings // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write document.getElementById('diff-auto-open').checked = message.data['diff.autoOpen'] !== false; + // Custom chat font (#44): applied via CSS custom properties only (never + // string-interpolated into CSS/HTML) so an arbitrary fontFamily value + // can't inject markup or styles. Empty/0 removes the property so the + // var() fallback in ui-styles.ts restores the editor default. + const chatFontFamily = message.data['ui.fontFamily']; + if (chatFontFamily && String(chatFontFamily).trim()) { + document.documentElement.style.setProperty('--chat-font-family', chatFontFamily); + } else { + document.documentElement.style.removeProperty('--chat-font-family'); + } + const chatFontSize = Number(message.data['ui.fontSize']) || 0; + if (chatFontSize > 0) { + const clampedChatFontSize = Math.min(72, Math.max(6, chatFontSize)); + document.documentElement.style.setProperty('--chat-font-size', clampedChatFontSize + 'px'); + } else { + document.documentElement.style.removeProperty('--chat-font-size'); + } + // Re-measure the input's inline height for the new font size, otherwise + // it keeps the old (possibly too small) height until the next keystroke. + adjustTextareaHeight(); const thinkingIntensity = message.data['thinking.intensity'] || 'think'; const intensityValues = ['think', 'think-hard', 'think-harder', 'ultrathink']; const sliderValue = intensityValues.indexOf(thinkingIntensity); diff --git a/src/ui-styles.ts b/src/ui-styles.ts index d05ae4e..a986a31 100644 --- a/src/ui-styles.ts +++ b/src/ui-styles.ts @@ -945,8 +945,8 @@ const styles = ` flex: 1; padding: 10px; overflow-y: auto; - font-family: var(--vscode-editor-font-family); - font-size: var(--vscode-editor-font-size); + font-family: var(--chat-font-family, var(--vscode-editor-font-family)); + font-size: var(--chat-font-size, var(--vscode-editor-font-size)); line-height: 1.4; } @@ -960,7 +960,7 @@ const styles = ` border: 1px solid rgba(64, 165, 255, 0.2); border-radius: 8px; color: var(--vscode-editor-foreground); - font-family: var(--vscode-editor-font-family); + font-family: var(--chat-font-family, var(--vscode-editor-font-family)); position: relative; overflow: hidden; } @@ -1039,7 +1039,7 @@ const styles = ` border: 1px solid rgba(28, 192, 140, 0.2); border-radius: 8px; color: var(--vscode-editor-foreground); - font-family: var(--vscode-editor-font-family); + font-family: var(--chat-font-family, var(--vscode-editor-font-family)); white-space: pre-wrap; position: relative; overflow: hidden; @@ -1059,7 +1059,7 @@ const styles = ` border: 1px solid rgba(186, 85, 211, 0.2); border-radius: 8px; color: var(--vscode-editor-foreground); - font-family: var(--vscode-editor-font-family); + font-family: var(--chat-font-family, var(--vscode-editor-font-family)); font-style: italic; opacity: 0.9; position: relative; @@ -1947,7 +1947,8 @@ const styles = ` border: none; padding: 12px; outline: none; - font-family: var(--vscode-editor-font-family); + font-family: var(--chat-font-family, var(--vscode-editor-font-family)); + font-size: var(--chat-font-size, inherit); min-height: 68px; line-height: 1.4; overflow-y: hidden; From 9d6b0081d1dc1f90a546fe2c83e912ca9fb353cf Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Sat, 25 Jul 2026 14:58:02 +0200 Subject: [PATCH 17/26] feat: expose font and max-output-token settings in the chat settings modal (fork-issue-44, fork-issue-42) The three settings already exist in the manifest; this surfaces them in the gear modal via the existing updateSettings/settingsData roundtrip, clamps them like the manifest does, and adds claudeCodeChat.advanced to the config-change broadcast so a second open surface cannot overwrite a fresh value with a stale one. Co-Authored-By: Claude Fable 5 --- src/extension.ts | 1 + src/script.ts | 30 +++++++++++++++++++++++++++--- src/ui.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index fab043c..4f84830 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3617,6 +3617,7 @@ class ClaudeChatProvider { 'permissions.yoloMode': config.get('permissions.yoloMode', false), 'router.enabled': config.get('router.enabled', false), 'executable.path': config.get('executable.path', ''), + 'advanced.maxOutputTokens': config.get('advanced.maxOutputTokens', 0), 'environment.variables': config.get>('environment.variables', {}), 'environment.disabled': config.get('environment.disabled', false), 'diff.autoOpen': config.get('diff.autoOpen', true), diff --git a/src/script.ts b/src/script.ts index cd2818a..95dc6f0 100644 --- a/src/script.ts +++ b/src/script.ts @@ -4926,9 +4926,24 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const wslClaudePath = document.getElementById('wsl-claude-path').value; const yoloMode = document.getElementById('yolo-mode').checked; const executablePath = document.getElementById('executable-path').value; + // #42/#44 settings modal follow-up: keep in sync with the manifest bounds + // (advanced.maxOutputTokens >= 0, ui.fontSize 0 or 6-72). + let maxOutputTokens = parseInt(document.getElementById('max-output-tokens').value, 10); + if (!Number.isFinite(maxOutputTokens) || maxOutputTokens < 0) { + maxOutputTokens = 0; + } const useRouter = document.getElementById('use-router')?.checked || false; // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write const diffAutoOpen = document.getElementById('diff-auto-open').checked; + const chatFontFamily = document.getElementById('chat-font-family').value; + let chatFontSize = parseInt(document.getElementById('chat-font-size').value, 10); + if (!Number.isFinite(chatFontSize) || chatFontSize < 0) { + chatFontSize = 0; + } else if (chatFontSize > 0 && chatFontSize < 6) { + chatFontSize = 6; + } else if (chatFontSize > 72) { + chatFontSize = 72; + } // Collect environment variables from key-value UI const envVariables = getEnvVariablesFromUI(); @@ -4967,9 +4982,12 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt 'wsl.claudePath': wslClaudePath || '/usr/local/bin/claude', 'permissions.yoloMode': yoloMode, 'executable.path': executablePath, + 'advanced.maxOutputTokens': maxOutputTokens, 'environment.variables': envVariables, 'router.enabled': useRouter, - 'diff.autoOpen': diffAutoOpen + 'diff.autoOpen': diffAutoOpen, + 'ui.fontFamily': chatFontFamily, + 'ui.fontSize': chatFontSize } }); } @@ -5239,12 +5257,16 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt document.documentElement.style.removeProperty('--chat-font-family'); } const chatFontSize = Number(message.data['ui.fontSize']) || 0; - if (chatFontSize > 0) { - const clampedChatFontSize = Math.min(72, Math.max(6, chatFontSize)); + const clampedChatFontSize = chatFontSize > 0 ? Math.min(72, Math.max(6, chatFontSize)) : 0; + if (clampedChatFontSize > 0) { document.documentElement.style.setProperty('--chat-font-size', clampedChatFontSize + 'px'); } else { document.documentElement.style.removeProperty('--chat-font-size'); } + // #44 settings modal: reflect the persisted values in the Appearance fields + // (clamped, so the field always shows the size that is actually applied) + document.getElementById('chat-font-family').value = chatFontFamily || ''; + document.getElementById('chat-font-size').value = clampedChatFontSize; // Re-measure the input's inline height for the new font size, otherwise // it keeps the old (possibly too small) height until the next keystroke. adjustTextareaHeight(); @@ -5282,6 +5304,8 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update Customize Claude Command settings document.getElementById('executable-path').value = message.data['executable.path'] || ''; + + document.getElementById('max-output-tokens').value = message.data['advanced.maxOutputTokens'] || 0; renderEnvVariables(message.data['environment.variables'] || {}); // Detect OpenCredits and envs disabled state diff --git a/src/ui.ts b/src/ui.ts index e1452ea..7b7c023 100644 --- a/src/ui.ts +++ b/src/ui.ts @@ -388,6 +388,14 @@ const getHtml = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'https

+
+ + +

+ Maximum number of tokens Claude may generate in a single response (sets CLAUDE_CODE_MAX_OUTPUT_TOKENS). Increase this if you hit a "response exceeded the output token maximum" error. 0 = use the CLI default. +

+
+
@@ -422,6 +430,24 @@ const getHtml = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'https
+

Appearance

+
+
+ + +

+ Custom font family for the chat message area and input field. Leave empty to use the editor's default font. +

+
+
+ + +

+ Custom font size (px, 6-72) for the chat message area and input field. Leave at 0 to use the editor's default font size. +

+
+
+ From 6ba6e262b43cee4dffb9df6ccb9b8dcaa0405bc5 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Mon, 27 Jul 2026 16:03:18 +0200 Subject: [PATCH 18/26] fix: keep saving the settings batch after one key fails (fork-issue-56) _updateSettings ran the whole settings batch from the webview in one loop wrapped in a single try/catch, so a rejected config.update() -- e.g. a setting not yet registered right after a version bump -- aborted the loop and silently dropped every key that came after it in the same batch. In the order the webview sends them, a rejection on e.g. advanced.maxOutputTokens would have dropped environment.variables, router.enabled, diff.autoOpen, ui.fontFamily and ui.fontSize with no indication in the UI. Each key now gets its own try/catch through the new vscode-free settings-batch module: failures are collected instead of aborting, _sendCurrentSettings() and the balance refresh run even on partial failure, and one summary error message names every failed key plus the first error's reason. Co-Authored-By: Claude Opus 5 --- package.json | 3 +- src/extension.ts | 21 ++++- src/settings-batch.ts | 84 ++++++++++++++++++ src/test/settings-batch.test.ts | 150 ++++++++++++++++++++++++++++++++ 4 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 src/settings-batch.ts create mode 100644 src/test/settings-batch.test.ts diff --git a/package.json b/package.json index 6a768a0..c8d7033 100644 --- a/package.json +++ b/package.json @@ -246,7 +246,8 @@ "test:downloader:unit": "npm run compile && mocha --ui tdd out/test/downloader.test.js --reporter spec", "test:models": "npm run compile && mocha --ui tdd out/test/model-updater.test.js --reporter spec", "test:diff-utils": "npm run compile && mocha --ui tdd out/test/diff-utils.test.js --reporter spec", - "test:restore-commit-utils": "npm run compile && mocha --ui tdd out/test/restore-commit-utils.test.js --reporter spec" + "test:restore-commit-utils": "npm run compile && mocha --ui tdd out/test/restore-commit-utils.test.js --reporter spec", + "test:settings-batch": "npm run compile && mocha --ui tdd out/test/settings-batch.test.js --reporter spec" }, "devDependencies": { "@types/mocha": "^10.0.10", diff --git a/src/extension.ts b/src/extension.ts index 4f84830..aac532f 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -11,6 +11,7 @@ import recommendedModels from './recommended-models.json'; import { downloadClaude, detectPlatform, DownloaderError } from './claudeDownloader'; import { mapWslPathToWindows, toWorkspaceRelativePath, isBinaryContent, buildTurnDiffUriParts, parseTurnDiffUriParts, turnDiffCacheKey } from './diff-utils'; import { isValidCommitSha, findRehydratedCommitInfo } from './restore-commit-utils'; +import { applySettingsBatch } from './settings-batch'; // OpenCredits environment configuration let OPENCREDITS_API_URL = 'https://ccc.api.opencredits.ai'; @@ -3657,7 +3658,11 @@ class ClaudeChatProvider { const config = vscode.workspace.getConfiguration('claudeCodeChat'); try { - for (const [key, value] of Object.entries(settings)) { + // #56: each key gets its own try/catch (inside applySettingsBatch) so one + // rejected config.update() -- e.g. a setting not yet registered right after + // a version bump -- no longer silently drops every key that comes after it + // in the same batch. + const result = await applySettingsBatch(settings, async (key, value) => { if (key === 'permissions.yoloMode') { // YOLO mode: try workspace first, fall back to global try { @@ -3669,8 +3674,9 @@ class ClaudeChatProvider { // Other settings are global (user-wide) await config.update(key, value, vscode.ConfigurationTarget.Global); } - } + }); + // #56: must run even when some keys above failed, not just on full success. // Re-send settings so webview gets updated isOpenCredits flag, etc. this._sendCurrentSettings(); @@ -3684,6 +3690,17 @@ class ClaudeChatProvider { balance: null }); } + + if (result.failures.length > 0) { + // One error: name it with its own message. Several: list every failed + // key, but still show the first error's message -- the "why" (e.g. a + // VS Code "not a registered configuration" message) is the actionable + // part, not just which keys failed. + const failedKeys = result.failures.map(f => f.key).join(', '); + const summary = `${failedKeys}: ${result.failures[0].message}`; + console.error('Failed to update settings:', result.failures); + vscode.window.showErrorMessage(`Failed to update settings: ${summary}`); + } } catch (error: any) { console.error('Failed to update settings:', error?.message || error); vscode.window.showErrorMessage(`Failed to update settings: ${error?.message || 'Unknown error'}`); diff --git a/src/settings-batch.ts b/src/settings-batch.ts new file mode 100644 index 0000000..97d169b --- /dev/null +++ b/src/settings-batch.ts @@ -0,0 +1,84 @@ +// Pure batch-update helper for the #56 fix: extension.ts's _updateSettings used to run +// the whole settings batch from the webview through a single loop wrapped in one +// try/catch -- if config.update() threw for one key (e.g. a setting not yet registered +// right after a version bump), the loop broke and every subsequent key in the same +// batch silently never got saved. Real-world hit on 2026-07-26: ui.renderMath wasn't +// registered yet, and six settings that came after it in the same batch (font family, +// font size, completion popup/sound, send-on-enter, diff.autoOpen) were dropped without +// any indication in the UI. This module owns only the per-key try/catch + result +// collection; extension.ts still owns every side effect (the actual +// vscode.workspace config.update() call, permissions.yoloMode's workspace-then-global +// fallback, _permLog, the summary error message) via the injected updateSetting +// callback -- no vscode import here, so this runs under plain mocha, same pattern as +// shell-utils/restore-commit-utils/perm-log-redact. Run with `npm run test:settings-batch`. +// +// Review follow-up: a first cut of this module logged all "update ok" lines +// after the whole batch settled, then all "update FAILED" lines -- that lost the perm +// log's per-key chronology extension.ts's own onDidChangeConfiguration listener depends +// on (config.update() fires refreshSettingsOnConfigChange -> _sendCurrentSettings, +// which writes its own settingsData perm-log line right after the key that triggered +// it -- the #23 colorblind-roundtrip diagnosis relied on that interleaving), and meant +// a batch that hangs or throws mid-loop left zero "ok" lines behind, i.e. no progress +// evidence in exactly the case that needs it most. onSettled below is called +// synchronously inside the same per-key try/catch as updateSetting, so a caller that +// logs from it gets one line per key, in the exact order keys were attempted -- same +// guarantee applied/failures already have, just not batched up first. + +export interface SettingUpdateFailure { + key: string; + message: string; +} + +export interface SettingsBatchResult { + applied: string[]; + failures: SettingUpdateFailure[]; +} + +// Turns whatever a rejected updateSetting() call threw into a plain string, the same way +// the pre-#56 code's 'err=' + (error?.message || error) string-concatenation did (Error +// instances and message-bearing objects use .message; anything else -- a thrown string, +// undefined, a plain object -- coerces the same way String() / template-literal +// interpolation would), so a caller like extension.ts's _permLog never has to guard +// against a missing .message itself. +function toErrorMessage(error: unknown): string { + if (typeof error === 'object' && error !== null && 'message' in error) { + const message = (error as { message: unknown }).message; + if (typeof message === 'string' && message) { + return message; + } + } + if (typeof error === 'string') { + return error; + } + return String(error); +} + +// Applies every [key, value] pair in settings via updateSetting, one at a time, each in +// its own try/catch -- unlike the pre-#56 single try/catch around the whole loop, a +// rejection for one key never stops the remaining keys from being attempted. Keys are +// attempted in the same order Object.entries(settings) always yields (insertion order +// for string keys), so applied/failures each preserve that order internally. onSettled +// (optional) is invoked synchronously right after each key's own try/catch resolves -- +// with no error argument on success, with the same normalized string toErrorMessage +// already put into failures[].message on failure -- so a caller can log/react per key +// without waiting for the whole batch and without re-deriving the error text itself. +export async function applySettingsBatch( + settings: { [key: string]: any }, + updateSetting: (key: string, value: any) => Promise, + onSettled?: (key: string, value: unknown, error?: unknown) => void +): Promise { + const applied: string[] = []; + const failures: SettingUpdateFailure[] = []; + for (const [key, value] of Object.entries(settings)) { + try { + await updateSetting(key, value); + applied.push(key); + onSettled?.(key, value); + } catch (error) { + const message = toErrorMessage(error); + failures.push({ key, message }); + onSettled?.(key, value, message); + } + } + return { applied, failures }; +} diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts new file mode 100644 index 0000000..f47913a --- /dev/null +++ b/src/test/settings-batch.test.ts @@ -0,0 +1,150 @@ +// Unit tests for the #56 settings-batch fix (applySettingsBatch). Pure (no vscode, no +// network, no filesystem access), so these run under plain mocha against the compiled +// out/ output -- same pattern as restore-commit-utils/perm-log-redact/markdown-restore. +// The first two suites are the actual regression coverage for the bug: a key that +// throws must not abort the keys after it, unlike the pre-#56 single try/catch loop +// (extension.ts's old _updateSettings, which broke out of the whole batch on the first +// config.update() rejection and only ever recorded that one failure). Run with +// `npm run test:settings-batch`. + +import * as assert from 'assert'; +import { applySettingsBatch } from '../settings-batch'; + +suite('settings-batch: applySettingsBatch (all keys succeed)', () => { + + test('every key is applied in order, with an empty failures list', async () => { + const seen: Array<[string, any]> = []; + const result = await applySettingsBatch( + { 'ui.compactMode': true, 'ui.fontSize': 14, 'wsl.distro': 'Ubuntu' }, + async (key, value) => { seen.push([key, value]); } + ); + assert.deepStrictEqual(result.applied, ['ui.compactMode', 'ui.fontSize', 'wsl.distro']); + assert.deepStrictEqual(result.failures, []); + assert.deepStrictEqual(seen, [['ui.compactMode', true], ['ui.fontSize', 14], ['wsl.distro', 'Ubuntu']], + 'updateSetting must still be called with the original key/value pairs'); + }); +}); + +suite('settings-batch: applySettingsBatch (#56 -- a failing key must not abort the rest)', () => { + + test('a key that throws is recorded as a failure, and every key after it is still applied', async () => { + const result = await applySettingsBatch( + { 'ui.renderMath': true, 'ui.fontFamily': 'monospace', 'ui.fontSize': 14, 'diff.autoOpen': true }, + async (key) => { + if (key === 'ui.renderMath') { + throw new Error('config not registered'); + } + } + ); + assert.deepStrictEqual(result.applied, ['ui.fontFamily', 'ui.fontSize', 'diff.autoOpen'], + 'keys after the failing one must still be applied, not silently dropped (the real #56 scenario)'); + assert.deepStrictEqual(result.failures, [{ key: 'ui.renderMath', message: 'config not registered' }]); + }); + + test('a failing key in the middle of the batch still lets both earlier and later keys succeed', async () => { + const result = await applySettingsBatch( + { first: 1, second: 2, third: 3 }, + async (key) => { + if (key === 'second') { + throw new Error('boom'); + } + } + ); + assert.deepStrictEqual(result.applied, ['first', 'third']); + assert.deepStrictEqual(result.failures, [{ key: 'second', message: 'boom' }]); + }); +}); + +suite('settings-batch: applySettingsBatch (multiple failing keys)', () => { + + test('all failures are collected, in the order they were attempted, applied keys unaffected', async () => { + const result = await applySettingsBatch( + { a: 1, b: 2, c: 3, d: 4 }, + async (key) => { + if (key === 'a' || key === 'c') { + throw new Error(`bad key ${key}`); + } + } + ); + assert.deepStrictEqual(result.applied, ['b', 'd']); + assert.deepStrictEqual(result.failures, [ + { key: 'a', message: 'bad key a' }, + { key: 'c', message: 'bad key c' } + ]); + }); +}); + +suite('settings-batch: applySettingsBatch (empty batch)', () => { + + test('an empty settings object resolves with empty applied/failures and never calls updateSetting', async () => { + let calls = 0; + const result = await applySettingsBatch({}, async () => { calls++; }); + assert.deepStrictEqual(result, { applied: [], failures: [] }); + assert.strictEqual(calls, 0); + }); +}); + +suite('settings-batch: applySettingsBatch (error normalization)', () => { + + test('an Error instance uses its .message', async () => { + const result = await applySettingsBatch({ k: 1 }, async () => { throw new Error('boom'); }); + assert.strictEqual(result.failures[0].message, 'boom'); + }); + + test('a thrown plain string is used as the message as-is', async () => { + // Promise.reject(...), not a "throw" statement, so a rejection with a + // non-Error value (deliberate here, to exercise toErrorMessage's + // non-Error branch) doesn't trip the no-throw-literal lint rule. + const result = await applySettingsBatch({ k: 1 }, () => Promise.reject('plain string error')); + assert.strictEqual(result.failures[0].message, 'plain string error'); + }); + + test('a thrown undefined is converted to the text "undefined", never left as an actual undefined value', async () => { + const result = await applySettingsBatch({ k: 1 }, () => Promise.reject(undefined)); + assert.strictEqual(result.failures[0].message, 'undefined'); + assert.strictEqual(typeof result.failures[0].message, 'string'); + }); + + test('a rejected plain object without a .message property still yields a string, not a throw', async () => { + const result = await applySettingsBatch({ k: 1 }, () => Promise.reject({ code: 'EFAIL' })); + assert.strictEqual(typeof result.failures[0].message, 'string'); + }); +}); + +suite('settings-batch: applySettingsBatch (onSettled hook, opus-review point 1 -- per-key perm-log chronology)', () => { + + test('onSettled fires once per key, right after it settles, in attempt order, with no error argument on success', async () => { + const events: Array<[string, unknown, unknown]> = []; + await applySettingsBatch( + { a: 1, b: 2 }, + async () => { /* always succeeds */ }, + (key, value, error) => { events.push([key, value, error]); } + ); + assert.deepStrictEqual(events, [['a', 1, undefined], ['b', 2, undefined]]); + }); + + test('onSettled receives the exact normalized message failures[] carries for a failing key, interleaved with the surrounding successes', async () => { + const events: Array<[string, unknown, unknown]> = []; + const result = await applySettingsBatch( + { a: 1, b: 2, c: 3 }, + async (key) => { + if (key === 'b') { + throw new Error('boom'); + } + }, + (key, value, error) => { events.push([key, value, error]); } + ); + assert.deepStrictEqual(events, [['a', 1, undefined], ['b', 2, 'boom'], ['c', 3, undefined]]); + assert.strictEqual(result.failures[0].message, 'boom'); + }); +}); + +suite('settings-batch: applySettingsBatch (malformed input -- the outer safety-net catch in extension.ts)', () => { + + test("a nullish settings object rejects instead of resolving silently -- the only way out of this module into a caller's outer try/catch (extension.ts's key= marker)", async () => { + await assert.rejects( + () => applySettingsBatch(undefined as any, async () => { /* never reached */ }), + /Cannot convert undefined or null to object/ + ); + }); +}); From c4863c29e05c7104fcdbcdbd0416fe96ddc57c29 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 19:38:35 +0200 Subject: [PATCH 19/26] test: scrub internal review note from settings-batch suite title --- src/test/settings-batch.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index f47913a..ea5b831 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -111,7 +111,7 @@ suite('settings-batch: applySettingsBatch (error normalization)', () => { }); }); -suite('settings-batch: applySettingsBatch (onSettled hook, opus-review point 1 -- per-key perm-log chronology)', () => { +suite('settings-batch: applySettingsBatch (onSettled hook, per-key perm-log chronology)', () => { test('onSettled fires once per key, right after it settles, in attempt order, with no error argument on success', async () => { const events: Array<[string, unknown, unknown]> = []; From a20f5e12cdcaf7b403c9fed50f7335c8a7df5803 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 20:49:17 +0200 Subject: [PATCH 20/26] fix: remove the dead onSettled param and its inaccurate rationale (fork-issue-56) applySettingsBatch's onSettled callback was never wired up by any real caller (extension.ts's _updateSettings only ever passes two arguments) -- only its own tests exercised it. The "review follow-up" comment that justified it was also wrong: _updateSettings writes no perm-log line, and the refreshSettingsOnConfigChange function it names does not exist anywhere in this codebase. Dropped the parameter, its tests, and the misleading comment. --- src/settings-batch.ts | 23 ++--------------------- src/test/settings-batch.test.ts | 28 ---------------------------- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/src/settings-batch.ts b/src/settings-batch.ts index 97d169b..1780d23 100644 --- a/src/settings-batch.ts +++ b/src/settings-batch.ts @@ -11,18 +11,6 @@ // fallback, _permLog, the summary error message) via the injected updateSetting // callback -- no vscode import here, so this runs under plain mocha, same pattern as // shell-utils/restore-commit-utils/perm-log-redact. Run with `npm run test:settings-batch`. -// -// Review follow-up: a first cut of this module logged all "update ok" lines -// after the whole batch settled, then all "update FAILED" lines -- that lost the perm -// log's per-key chronology extension.ts's own onDidChangeConfiguration listener depends -// on (config.update() fires refreshSettingsOnConfigChange -> _sendCurrentSettings, -// which writes its own settingsData perm-log line right after the key that triggered -// it -- the #23 colorblind-roundtrip diagnosis relied on that interleaving), and meant -// a batch that hangs or throws mid-loop left zero "ok" lines behind, i.e. no progress -// evidence in exactly the case that needs it most. onSettled below is called -// synchronously inside the same per-key try/catch as updateSetting, so a caller that -// logs from it gets one line per key, in the exact order keys were attempted -- same -// guarantee applied/failures already have, just not batched up first. export interface SettingUpdateFailure { key: string; @@ -57,15 +45,10 @@ function toErrorMessage(error: unknown): string { // its own try/catch -- unlike the pre-#56 single try/catch around the whole loop, a // rejection for one key never stops the remaining keys from being attempted. Keys are // attempted in the same order Object.entries(settings) always yields (insertion order -// for string keys), so applied/failures each preserve that order internally. onSettled -// (optional) is invoked synchronously right after each key's own try/catch resolves -- -// with no error argument on success, with the same normalized string toErrorMessage -// already put into failures[].message on failure -- so a caller can log/react per key -// without waiting for the whole batch and without re-deriving the error text itself. +// for string keys), so applied/failures each preserve that order internally. export async function applySettingsBatch( settings: { [key: string]: any }, - updateSetting: (key: string, value: any) => Promise, - onSettled?: (key: string, value: unknown, error?: unknown) => void + updateSetting: (key: string, value: any) => Promise ): Promise { const applied: string[] = []; const failures: SettingUpdateFailure[] = []; @@ -73,11 +56,9 @@ export async function applySettingsBatch( try { await updateSetting(key, value); applied.push(key); - onSettled?.(key, value); } catch (error) { const message = toErrorMessage(error); failures.push({ key, message }); - onSettled?.(key, value, message); } } return { applied, failures }; diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index ea5b831..fc2dc72 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -111,34 +111,6 @@ suite('settings-batch: applySettingsBatch (error normalization)', () => { }); }); -suite('settings-batch: applySettingsBatch (onSettled hook, per-key perm-log chronology)', () => { - - test('onSettled fires once per key, right after it settles, in attempt order, with no error argument on success', async () => { - const events: Array<[string, unknown, unknown]> = []; - await applySettingsBatch( - { a: 1, b: 2 }, - async () => { /* always succeeds */ }, - (key, value, error) => { events.push([key, value, error]); } - ); - assert.deepStrictEqual(events, [['a', 1, undefined], ['b', 2, undefined]]); - }); - - test('onSettled receives the exact normalized message failures[] carries for a failing key, interleaved with the surrounding successes', async () => { - const events: Array<[string, unknown, unknown]> = []; - const result = await applySettingsBatch( - { a: 1, b: 2, c: 3 }, - async (key) => { - if (key === 'b') { - throw new Error('boom'); - } - }, - (key, value, error) => { events.push([key, value, error]); } - ); - assert.deepStrictEqual(events, [['a', 1, undefined], ['b', 2, 'boom'], ['c', 3, undefined]]); - assert.strictEqual(result.failures[0].message, 'boom'); - }); -}); - suite('settings-batch: applySettingsBatch (malformed input -- the outer safety-net catch in extension.ts)', () => { test("a nullish settings object rejects instead of resolving silently -- the only way out of this module into a caller's outer try/catch (extension.ts's key= marker)", async () => { From d8f966f88121ad37652c792585a7bdcff64f319f Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 20:51:07 +0200 Subject: [PATCH 21/26] chore: de-link bare internal issue references reintroduced by the settings-modal commits Bare #NN in comments/suite titles auto-links to this fork's own GitHub issues instead of our private tracker. Requalified as fork-issue-NN in extension.ts, script.ts, settings-batch.ts and its test suite (including the suite title, which shows up verbatim in the mocha reporter output). Genuine "upstream #150" references are left untouched. --- src/extension.ts | 8 ++++---- src/script.ts | 8 ++++---- src/settings-batch.ts | 6 +++--- src/test/settings-batch.test.ts | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index aac532f..f90e0bc 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -786,7 +786,7 @@ class ClaudeChatProvider { this._enableYoloMode(); return; case 'openMaxOutputTokensSettings': - // #42: deep-link into the native Settings UI, filtered on our setting. + // fork-issue-42: deep-link into the native Settings UI, filtered on our setting. // No value is set automatically - the user picks the limit themselves. vscode.commands.executeCommand('workbench.action.openSettings', 'claudeCodeChat.advanced.maxOutputTokens'); return; @@ -1124,7 +1124,7 @@ class ClaudeChatProvider { NO_COLOR: '1', ...customEnvVars, // Apply custom environment variables (ANTHROPIC_AUTH_TOKEN, ANTHROPIC_BASE_URL, etc.) CLAUDE_CODE_ENTRYPOINT: 'claude-vscode', - // #42: raise the CLI's response size cap when configured, to work around + // fork-issue-42: raise the CLI's response size cap when configured, to work around // "response exceeded the output token maximum" errors (upstream #150) ...(Math.floor(maxOutputTokens) > 0 ? { CLAUDE_CODE_MAX_OUTPUT_TOKENS: String(Math.floor(maxOutputTokens)) } : {}) }; @@ -3658,7 +3658,7 @@ class ClaudeChatProvider { const config = vscode.workspace.getConfiguration('claudeCodeChat'); try { - // #56: each key gets its own try/catch (inside applySettingsBatch) so one + // fork-issue-56: each key gets its own try/catch (inside applySettingsBatch) so one // rejected config.update() -- e.g. a setting not yet registered right after // a version bump -- no longer silently drops every key that comes after it // in the same batch. @@ -3676,7 +3676,7 @@ class ClaudeChatProvider { } }); - // #56: must run even when some keys above failed, not just on full success. + // fork-issue-56: must run even when some keys above failed, not just on full success. // Re-send settings so webview gets updated isOpenCredits flag, etc. this._sendCurrentSettings(); diff --git a/src/script.ts b/src/script.ts index 95dc6f0..df08fb6 100644 --- a/src/script.ts +++ b/src/script.ts @@ -189,7 +189,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } // Check if this is an output token limit error and offer a shortcut to - // raise CLAUDE_CODE_MAX_OUTPUT_TOKENS via the setting (#42, upstream #150) + // raise CLAUDE_CODE_MAX_OUTPUT_TOKENS via the setting (fork-issue-42, upstream #150) if ((type === 'error' || type === 'claude') && isOutputTokenLimitError(content)) { const tokenLimitSuggestion = document.createElement('div'); tokenLimitSuggestion.className = 'yolo-suggestion'; @@ -4926,7 +4926,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const wslClaudePath = document.getElementById('wsl-claude-path').value; const yoloMode = document.getElementById('yolo-mode').checked; const executablePath = document.getElementById('executable-path').value; - // #42/#44 settings modal follow-up: keep in sync with the manifest bounds + // fork-issue-42/fork-issue-44 settings modal follow-up: keep in sync with the manifest bounds // (advanced.maxOutputTokens >= 0, ui.fontSize 0 or 6-72). let maxOutputTokens = parseInt(document.getElementById('max-output-tokens').value, 10); if (!Number.isFinite(maxOutputTokens) || maxOutputTokens < 0) { @@ -5246,7 +5246,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt // Update UI with current settings // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write document.getElementById('diff-auto-open').checked = message.data['diff.autoOpen'] !== false; - // Custom chat font (#44): applied via CSS custom properties only (never + // Custom chat font (fork-issue-44): applied via CSS custom properties only (never // string-interpolated into CSS/HTML) so an arbitrary fontFamily value // can't inject markup or styles. Empty/0 removes the property so the // var() fallback in ui-styles.ts restores the editor default. @@ -5263,7 +5263,7 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt } else { document.documentElement.style.removeProperty('--chat-font-size'); } - // #44 settings modal: reflect the persisted values in the Appearance fields + // fork-issue-44 settings modal: reflect the persisted values in the Appearance fields // (clamped, so the field always shows the size that is actually applied) document.getElementById('chat-font-family').value = chatFontFamily || ''; document.getElementById('chat-font-size').value = clampedChatFontSize; diff --git a/src/settings-batch.ts b/src/settings-batch.ts index 1780d23..e067233 100644 --- a/src/settings-batch.ts +++ b/src/settings-batch.ts @@ -1,4 +1,4 @@ -// Pure batch-update helper for the #56 fix: extension.ts's _updateSettings used to run +// Pure batch-update helper for the fork-issue-56 fix: extension.ts's _updateSettings used to run // the whole settings batch from the webview through a single loop wrapped in one // try/catch -- if config.update() threw for one key (e.g. a setting not yet registered // right after a version bump), the loop broke and every subsequent key in the same @@ -23,7 +23,7 @@ export interface SettingsBatchResult { } // Turns whatever a rejected updateSetting() call threw into a plain string, the same way -// the pre-#56 code's 'err=' + (error?.message || error) string-concatenation did (Error +// the pre-fork-issue-56 code's 'err=' + (error?.message || error) string-concatenation did (Error // instances and message-bearing objects use .message; anything else -- a thrown string, // undefined, a plain object -- coerces the same way String() / template-literal // interpolation would), so a caller like extension.ts's _permLog never has to guard @@ -42,7 +42,7 @@ function toErrorMessage(error: unknown): string { } // Applies every [key, value] pair in settings via updateSetting, one at a time, each in -// its own try/catch -- unlike the pre-#56 single try/catch around the whole loop, a +// its own try/catch -- unlike the pre-fork-issue-56 single try/catch around the whole loop, a // rejection for one key never stops the remaining keys from being attempted. Keys are // attempted in the same order Object.entries(settings) always yields (insertion order // for string keys), so applied/failures each preserve that order internally. diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index fc2dc72..d4da82a 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -1,8 +1,8 @@ -// Unit tests for the #56 settings-batch fix (applySettingsBatch). Pure (no vscode, no +// Unit tests for the fork-issue-56 settings-batch fix (applySettingsBatch). Pure (no vscode, no // network, no filesystem access), so these run under plain mocha against the compiled // out/ output -- same pattern as restore-commit-utils/perm-log-redact/markdown-restore. // The first two suites are the actual regression coverage for the bug: a key that -// throws must not abort the keys after it, unlike the pre-#56 single try/catch loop +// throws must not abort the keys after it, unlike the pre-fork-issue-56 single try/catch loop // (extension.ts's old _updateSettings, which broke out of the whole batch on the first // config.update() rejection and only ever recorded that one failure). Run with // `npm run test:settings-batch`. @@ -25,7 +25,7 @@ suite('settings-batch: applySettingsBatch (all keys succeed)', () => { }); }); -suite('settings-batch: applySettingsBatch (#56 -- a failing key must not abort the rest)', () => { +suite('settings-batch: applySettingsBatch (fork-issue-56 -- a failing key must not abort the rest)', () => { test('a key that throws is recorded as a failure, and every key after it is still applied', async () => { const result = await applySettingsBatch( @@ -37,7 +37,7 @@ suite('settings-batch: applySettingsBatch (#56 -- a failing key must not abort t } ); assert.deepStrictEqual(result.applied, ['ui.fontFamily', 'ui.fontSize', 'diff.autoOpen'], - 'keys after the failing one must still be applied, not silently dropped (the real #56 scenario)'); + 'keys after the failing one must still be applied, not silently dropped (the real fork-issue-56 scenario)'); assert.deepStrictEqual(result.failures, [{ key: 'ui.renderMath', message: 'config not registered' }]); }); From f27ea8d3c78a502f478e8273f2e91abe541ad3ac Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 20:51:23 +0200 Subject: [PATCH 22/26] docs: correct 6257620's overstated config-change-broadcast claim That commit's message says it "adds claudeCodeChat.advanced to the config-change broadcast", but it only adds a key to the plain settingsData payload _sendCurrentSettings already sends. There is no onDidChangeConfiguration/affectsConfiguration listener for it -- the only one (activate(), above) still filters on claudeCodeChat.wsl only. 6257620 isn't the tip of this stack, so a code comment corrects the record instead of an amend. --- src/extension.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/extension.ts b/src/extension.ts index f90e0bc..627dd72 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3618,6 +3618,11 @@ class ClaudeChatProvider { 'permissions.yoloMode': config.get('permissions.yoloMode', false), 'router.enabled': config.get('router.enabled', false), 'executable.path': config.get('executable.path', ''), + // Correction to the fork-issue-44/fork-issue-42 commit message: this line only adds + // the key to the plain settingsData payload _sendCurrentSettings already sends -- + // there is no claudeCodeChat.advanced entry in any onDidChangeConfiguration / + // affectsConfiguration listener (the only one, above in activate(), still filters + // on claudeCodeChat.wsl only). 'advanced.maxOutputTokens': config.get('advanced.maxOutputTokens', 0), 'environment.variables': config.get>('environment.variables', {}), 'environment.disabled': config.get('environment.disabled', false), From 90ccc941a2485dab066ed07cf677411a74594c60 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 21:18:55 +0200 Subject: [PATCH 23/26] docs: fix settings-batch comments' false _permLog claim and stale sibling-file refs The module comment and toErrorMessage's comment both claimed extension.ts's injected updateSetting callback (and _updateSettings) call _permLog as a side effect; neither does -- _permLog's only call sites (extension.ts:3893, 3971, 4304) are the unrelated usageLimits/turndiff logging. Named the real caller (_updateSettings) instead. Both files also pointed at "shell-utils" and "perm-log-redact"/ "markdown-restore" as sibling test-pattern files; none of those exist in this branch, only restore-commit-utils does. Corrected the references, and fixed the malformed-input test's title, which named a "extension.ts's key= marker" that doesn't exist -- the actual outer catch in _updateSettings just logs and shows an error message. --- src/settings-batch.ts | 8 ++++---- src/test/settings-batch.test.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/settings-batch.ts b/src/settings-batch.ts index e067233..16bd19c 100644 --- a/src/settings-batch.ts +++ b/src/settings-batch.ts @@ -8,9 +8,9 @@ // any indication in the UI. This module owns only the per-key try/catch + result // collection; extension.ts still owns every side effect (the actual // vscode.workspace config.update() call, permissions.yoloMode's workspace-then-global -// fallback, _permLog, the summary error message) via the injected updateSetting +// fallback, the summary error message) via the injected updateSetting // callback -- no vscode import here, so this runs under plain mocha, same pattern as -// shell-utils/restore-commit-utils/perm-log-redact. Run with `npm run test:settings-batch`. +// restore-commit-utils. Run with `npm run test:settings-batch`. export interface SettingUpdateFailure { key: string; @@ -26,8 +26,8 @@ export interface SettingsBatchResult { // the pre-fork-issue-56 code's 'err=' + (error?.message || error) string-concatenation did (Error // instances and message-bearing objects use .message; anything else -- a thrown string, // undefined, a plain object -- coerces the same way String() / template-literal -// interpolation would), so a caller like extension.ts's _permLog never has to guard -// against a missing .message itself. +// interpolation would), so a caller like extension.ts's _updateSettings never has to +// guard against a missing .message itself. function toErrorMessage(error: unknown): string { if (typeof error === 'object' && error !== null && 'message' in error) { const message = (error as { message: unknown }).message; diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index d4da82a..20e96f7 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -1,6 +1,6 @@ // Unit tests for the fork-issue-56 settings-batch fix (applySettingsBatch). Pure (no vscode, no // network, no filesystem access), so these run under plain mocha against the compiled -// out/ output -- same pattern as restore-commit-utils/perm-log-redact/markdown-restore. +// out/ output -- same pattern as restore-commit-utils. // The first two suites are the actual regression coverage for the bug: a key that // throws must not abort the keys after it, unlike the pre-fork-issue-56 single try/catch loop // (extension.ts's old _updateSettings, which broke out of the whole batch on the first @@ -113,7 +113,7 @@ suite('settings-batch: applySettingsBatch (error normalization)', () => { suite('settings-batch: applySettingsBatch (malformed input -- the outer safety-net catch in extension.ts)', () => { - test("a nullish settings object rejects instead of resolving silently -- the only way out of this module into a caller's outer try/catch (extension.ts's key= marker)", async () => { + test("a nullish settings object rejects instead of resolving silently -- the only way out of this module into a caller's outer try/catch (extension.ts's _updateSettings, which logs and shows an error message)", async () => { await assert.rejects( () => applySettingsBatch(undefined as any, async () => { /* never reached */ }), /Cannot convert undefined or null to object/ From fcc7066d3d22602dc51986c561118f841df65996 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Tue, 28 Jul 2026 21:57:22 +0200 Subject: [PATCH 24/26] docs: replace fabricated ui.renderMath scenario with real setting keys settings-batch.ts's module comment and the matching test fixture both invented a "real-world hit on 2026-07-26" involving ui.renderMath and six settings (font family/size, completion popup/sound, send-on-enter) that don't exist anywhere in this branch's package.json -- the only settings registered here are wsl.*, thinking.intensity, permissions.yoloMode, executable.path, advanced.maxOutputTokens, environment.*, router.enabled, diff.autoOpen, ui.fontFamily and ui.fontSize. Replaced the illustration with the actual key order _updateSettings receives from the webview, so a rejection on advanced.maxOutputTokens is shown dropping the real keys that follow it (environment.variables, router.enabled, diff.autoOpen, ui.fontFamily, ui.fontSize). Updated the test fixture accordingly. --- src/settings-batch.ts | 11 ++++++----- src/test/settings-batch.test.ts | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/settings-batch.ts b/src/settings-batch.ts index 16bd19c..ae13b00 100644 --- a/src/settings-batch.ts +++ b/src/settings-batch.ts @@ -2,11 +2,12 @@ // the whole settings batch from the webview through a single loop wrapped in one // try/catch -- if config.update() threw for one key (e.g. a setting not yet registered // right after a version bump), the loop broke and every subsequent key in the same -// batch silently never got saved. Real-world hit on 2026-07-26: ui.renderMath wasn't -// registered yet, and six settings that came after it in the same batch (font family, -// font size, completion popup/sound, send-on-enter, diff.autoOpen) were dropped without -// any indication in the UI. This module owns only the per-key try/catch + result -// collection; extension.ts still owns every side effect (the actual +// batch silently never got saved. In the order the webview's updateSettings() message +// sends them, a rejection on e.g. advanced.maxOutputTokens would have silently dropped +// every key after it in the same batch (environment.variables, router.enabled, +// diff.autoOpen, ui.fontFamily, ui.fontSize) without any indication in the UI. This +// module owns only the per-key try/catch + result collection; extension.ts still owns +// every side effect (the actual // vscode.workspace config.update() call, permissions.yoloMode's workspace-then-global // fallback, the summary error message) via the injected updateSetting // callback -- no vscode import here, so this runs under plain mocha, same pattern as diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index 20e96f7..bc80481 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -29,16 +29,16 @@ suite('settings-batch: applySettingsBatch (fork-issue-56 -- a failing key must n test('a key that throws is recorded as a failure, and every key after it is still applied', async () => { const result = await applySettingsBatch( - { 'ui.renderMath': true, 'ui.fontFamily': 'monospace', 'ui.fontSize': 14, 'diff.autoOpen': true }, + { 'advanced.maxOutputTokens': 5000, 'ui.fontFamily': 'monospace', 'ui.fontSize': 14, 'diff.autoOpen': true }, async (key) => { - if (key === 'ui.renderMath') { + if (key === 'advanced.maxOutputTokens') { throw new Error('config not registered'); } } ); assert.deepStrictEqual(result.applied, ['ui.fontFamily', 'ui.fontSize', 'diff.autoOpen'], - 'keys after the failing one must still be applied, not silently dropped (the real fork-issue-56 scenario)'); - assert.deepStrictEqual(result.failures, [{ key: 'ui.renderMath', message: 'config not registered' }]); + 'keys after the failing one must still be applied, not silently dropped (the fork-issue-56 scenario)'); + assert.deepStrictEqual(result.failures, [{ key: 'advanced.maxOutputTokens', message: 'config not registered' }]); }); test('a failing key in the middle of the batch still lets both earlier and later keys succeed', async () => { From bb5ee6a10c28038918b109f4f6f3cdd37eb98c4c Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Wed, 29 Jul 2026 05:41:07 +0200 Subject: [PATCH 25/26] test: replace the nonexistent ui.compactMode fixture key with a real setting settings-batch.test.ts's first suite plumbed 'ui.compactMode' through applySettingsBatch, but that setting doesn't exist in this branch's package.json (only wsl.*, thinking.intensity, permissions.yoloMode, executable.path, advanced.maxOutputTokens, environment.*, router.enabled, diff.autoOpen, ui.fontFamily and ui.fontSize do). Swapped it for advanced.maxOutputTokens, already used the same way in the second suite below. Co-Authored-By: Claude Sonnet 5 --- src/test/settings-batch.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/settings-batch.test.ts b/src/test/settings-batch.test.ts index bc80481..6a7ea3b 100644 --- a/src/test/settings-batch.test.ts +++ b/src/test/settings-batch.test.ts @@ -15,12 +15,12 @@ suite('settings-batch: applySettingsBatch (all keys succeed)', () => { test('every key is applied in order, with an empty failures list', async () => { const seen: Array<[string, any]> = []; const result = await applySettingsBatch( - { 'ui.compactMode': true, 'ui.fontSize': 14, 'wsl.distro': 'Ubuntu' }, + { 'advanced.maxOutputTokens': 5000, 'ui.fontSize': 14, 'wsl.distro': 'Ubuntu' }, async (key, value) => { seen.push([key, value]); } ); - assert.deepStrictEqual(result.applied, ['ui.compactMode', 'ui.fontSize', 'wsl.distro']); + assert.deepStrictEqual(result.applied, ['advanced.maxOutputTokens', 'ui.fontSize', 'wsl.distro']); assert.deepStrictEqual(result.failures, []); - assert.deepStrictEqual(seen, [['ui.compactMode', true], ['ui.fontSize', 14], ['wsl.distro', 'Ubuntu']], + assert.deepStrictEqual(seen, [['advanced.maxOutputTokens', 5000], ['ui.fontSize', 14], ['wsl.distro', 'Ubuntu']], 'updateSetting must still be called with the original key/value pairs'); }); }); From 75237b7f1074b58fa0603777c47093c7db5345b3 Mon Sep 17 00:00:00 2001 From: Jonas Kunert Date: Wed, 29 Jul 2026 05:41:16 +0200 Subject: [PATCH 26/26] fix: don't zero out max-output-tokens/font settings when the modal was never opened enableYoloMode() (the button on every permission-error banner) flips the yolo-mode checkbox and calls updateSettings(), which now also reads max-output-tokens, chat-font-family and chat-font-size. Those three fields are only populated by the settingsData roundtrip that runs when the gear modal opens; clicking Enable Yolo Mode without ever opening the modal left them at their empty '' default, so parseInt('') -> NaN got clamped to 0 above and was sent as advanced.maxOutputTokens/ui.fontSize, silently resetting a real value back to 0. updateSettings() now only adds these three keys to the settings batch it sends when their DOM element actually holds a value. Co-Authored-By: Claude Sonnet 5 --- src/script.ts | 50 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/script.ts b/src/script.ts index df08fb6..3085ab5 100644 --- a/src/script.ts +++ b/src/script.ts @@ -4928,15 +4928,18 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt const executablePath = document.getElementById('executable-path').value; // fork-issue-42/fork-issue-44 settings modal follow-up: keep in sync with the manifest bounds // (advanced.maxOutputTokens >= 0, ui.fontSize 0 or 6-72). - let maxOutputTokens = parseInt(document.getElementById('max-output-tokens').value, 10); + const maxOutputTokensEl = document.getElementById('max-output-tokens'); + let maxOutputTokens = parseInt(maxOutputTokensEl.value, 10); if (!Number.isFinite(maxOutputTokens) || maxOutputTokens < 0) { maxOutputTokens = 0; } const useRouter = document.getElementById('use-router')?.checked || false; // fork-issue-38: auto-open a turn diff after a successful Edit/MultiEdit/Write const diffAutoOpen = document.getElementById('diff-auto-open').checked; - const chatFontFamily = document.getElementById('chat-font-family').value; - let chatFontSize = parseInt(document.getElementById('chat-font-size').value, 10); + const chatFontFamilyEl = document.getElementById('chat-font-family'); + const chatFontFamily = chatFontFamilyEl.value; + const chatFontSizeEl = document.getElementById('chat-font-size'); + let chatFontSize = parseInt(chatFontSizeEl.value, 10); if (!Number.isFinite(chatFontSize) || chatFontSize < 0) { chatFontSize = 0; } else if (chatFontSize > 0 && chatFontSize < 6) { @@ -4973,22 +4976,35 @@ const getScript = (isTelemetryEnabled: boolean, opencreditsApiUrl: string = 'htt has_custom_envs: Object.keys(envVariables).length > 0, has_custom_executable: !!executablePath }); + const settingsToSend = { + 'wsl.enabled': wslEnabled, + 'wsl.distro': wslDistro || 'Ubuntu', + 'wsl.nodePath': wslNodePath, + 'wsl.claudePath': wslClaudePath || '/usr/local/bin/claude', + 'permissions.yoloMode': yoloMode, + 'executable.path': executablePath, + 'environment.variables': envVariables, + 'router.enabled': useRouter, + 'diff.autoOpen': diffAutoOpen + }; + // The settings modal's settingsData roundtrip is what fills these three fields in; + // callers that trigger updateSettings() without ever opening the modal (e.g. + // enableYoloMode() from the permission-error banner) find them at their untouched '' + // default, so only send them once they actually hold a value -- otherwise + // parseInt('') -> NaN gets clamped to 0 above and would silently zero out a real + // advanced.maxOutputTokens/ui.fontSize setting. + if (maxOutputTokensEl.value !== '') { + settingsToSend['advanced.maxOutputTokens'] = maxOutputTokens; + } + if (chatFontFamilyEl.value !== '') { + settingsToSend['ui.fontFamily'] = chatFontFamily; + } + if (chatFontSizeEl.value !== '') { + settingsToSend['ui.fontSize'] = chatFontSize; + } vscode.postMessage({ type: 'updateSettings', - settings: { - 'wsl.enabled': wslEnabled, - 'wsl.distro': wslDistro || 'Ubuntu', - 'wsl.nodePath': wslNodePath, - 'wsl.claudePath': wslClaudePath || '/usr/local/bin/claude', - 'permissions.yoloMode': yoloMode, - 'executable.path': executablePath, - 'advanced.maxOutputTokens': maxOutputTokens, - 'environment.variables': envVariables, - 'router.enabled': useRouter, - 'diff.autoOpen': diffAutoOpen, - 'ui.fontFamily': chatFontFamily, - 'ui.fontSize': chatFontSize - } + settings: settingsToSend }); }