Skip to content

Commit 41162cb

Browse files
committed
fix(stats): resolve blockers B1/B2/B3 and highs H1/H3
1 parent d1f2064 commit 41162cb

27 files changed

Lines changed: 224 additions & 55 deletions

packages/types/src/__tests__/usage-stats.spec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ describe("usage-stats schemas", () => {
117117
})
118118

119119
it("should reject missing semantics", () => {
120-
const { semantics, ...withoutSemantics } = validEvent
120+
const { semantics: _semantics, ...withoutSemantics } = validEvent
121121
expect(() => UsageEventV1.parse(withoutSemantics)).toThrow()
122122
})
123123

@@ -126,7 +126,7 @@ describe("usage-stats schemas", () => {
126126
})
127127

128128
it("should reject missing required fields (eventId)", () => {
129-
const { eventId, ...withoutEventId } = validEvent
129+
const { eventId: _eventId, ...withoutEventId } = validEvent
130130
expect(() => UsageEventV1.parse(withoutEventId)).toThrow()
131131
})
132132

@@ -242,7 +242,7 @@ describe("usage-stats schemas", () => {
242242
})
243243

244244
it("should reject missing required numeric field", () => {
245-
const { costUsd, ...withoutCost } = validBucket
245+
const { costUsd: _costUsd, ...withoutCost } = validBucket
246246
expect(() => StatsBucket.parse(withoutCost)).toThrow()
247247
})
248248

@@ -311,12 +311,12 @@ describe("usage-stats schemas", () => {
311311
})
312312

313313
it("should reject missing coverage", () => {
314-
const { coverage, ...withoutCoverage } = validSnapshot
314+
const { coverage: _coverage, ...withoutCoverage } = validSnapshot
315315
expect(() => StatsSnapshot.parse(withoutCoverage)).toThrow()
316316
})
317317

318318
it("should reject missing totals", () => {
319-
const { totals, ...withoutTotals } = validSnapshot
319+
const { totals: _totals, ...withoutTotals } = validSnapshot
320320
expect(() => StatsSnapshot.parse(withoutTotals)).toThrow()
321321
})
322322
})

packages/types/src/vscode-extension-host.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ export interface ExtensionMessage {
108108
| "getUsageStatsResponse"
109109
| "clearUsageStatsResponse"
110110
| "exportUsageStatsResponse"
111+
| "requestClearNonceResponse"
111112
| "usageStatsChanged"
112113
text?: string
113114
/** For fileContent: { path, content, error? } */
@@ -258,6 +259,9 @@ export interface ExtensionMessage {
258259
usageStatsSnapshot?: StatsSnapshot
259260
clearUsageStatsResult?: { success: boolean; error?: string }
260261
exportUsageStatsResult?: { format: "json" | "csv"; data: string; error?: string }
262+
// B2 fix: host-issued clear nonce returned in `requestClearNonceResponse`.
263+
// null when the service is unavailable or an error occurred (see `error`).
264+
clearNonce?: string | null
261265
}
262266

263267
export interface OpenAiCodexRateLimitsMessage {
@@ -634,6 +638,7 @@ export interface WebviewMessage {
634638
| "getUsageStats"
635639
| "clearUsageStats"
636640
| "exportUsageStats"
641+
| "requestClearNonce"
637642
text?: string
638643
taskId?: string
639644
editedMessageContent?: string
@@ -748,6 +753,11 @@ export interface WebviewMessage {
748753
usageStatsQuery?: StatsQuery
749754
clearUsageStatsNonce?: string
750755
exportUsageStatsFormat?: "json" | "csv"
756+
// B2 fix: host-issued clear nonce returned to webview in response to
757+
// `requestClearNonce`. The webview must use this nonce (not a self-generated
758+
// one) when sending the subsequent `clearUsageStats` message, so the host's
759+
// nonce validation actually passes.
760+
clearNonce?: string
751761
}
752762

753763
export interface RequestOpenAiCodexRateLimitsMessage {

src/core/task/Task.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3140,7 +3140,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
31403140
// API attempts. We record the final usage event here.
31413141
// (Architecture report section 5.5-5.8: terminal finalize only, no chunk-level append)
31423142
if (this.usageRecorder) {
3143-
const requestKey = `${this.taskId}:${currentItem.retryAttempt ?? 0}`
3143+
// B1 fix: include apiReqIndex so each tool-use turn produces a unique
3144+
// requestKey. Previously requestKey = taskId:retryAttempt, which was
3145+
// identical for every turn of a task (retryAttempt resets to 0 per turn),
3146+
// causing the idempotency dedupe to drop all but the first turn's usage.
3147+
const requestKey = `${this.taskId}:${apiReqIndex}:${currentItem.retryAttempt ?? 0}`
31443148
const ctx: UsageRecordingContext = {
31453149
taskId: this.taskId,
31463150
parentTaskId: this.parentTaskId,
@@ -3284,7 +3288,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
32843288
// user cancellations. Record the partial usage with the appropriate status.
32853289
// (Architecture report section 5.5-5.8: terminal finalize only)
32863290
if (this.usageRecorder) {
3287-
const requestKey = `${this.taskId}:${currentItem.retryAttempt ?? 0}`
3291+
// B1 fix: include apiReqIndex so each tool-use turn produces a unique
3292+
// requestKey (see completed-path comment above).
3293+
const requestKey = `${this.taskId}:${lastApiReqIndex}:${currentItem.retryAttempt ?? 0}`
32883294
const failedStatus: "failed" | "cancelled" = this.abort ? "cancelled" : "failed"
32893295
const ctx: UsageRecordingContext = {
32903296
taskId: this.taskId,

src/core/webview/__tests__/usageStatsMessageHandler.spec.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -538,22 +538,41 @@ describe("usageStatsMessageHandler", () => {
538538
// ── handleRequestClearNonce ──────────────────────────────────────────────
539539

540540
describe("handleRequestClearNonce", () => {
541-
it("returns nonce from service", async () => {
541+
it("posts requestClearNonceResponse with nonce from service", async () => {
542542
const issueClearNonce = vi.fn(() => "test-nonce-abc")
543543
const provider = createMockProvider({ issueClearNonce })
544544

545-
const result = await handleRequestClearNonce(provider)
545+
const message: WebviewMessage = {
546+
type: "requestClearNonce",
547+
requestId: "req-nonce-1",
548+
}
549+
550+
await handleRequestClearNonce(provider, message)
546551

547552
expect(issueClearNonce).toHaveBeenCalled()
548-
expect(result).toBe("test-nonce-abc")
553+
expect(provider.postMessageToWebview).toHaveBeenCalledWith({
554+
type: "requestClearNonceResponse",
555+
requestId: "req-nonce-1",
556+
clearNonce: "test-nonce-abc",
557+
})
549558
})
550559

551-
it("returns null when service is unavailable", async () => {
560+
it("posts error response when service is unavailable", async () => {
552561
const provider = createMockProvider(undefined)
553562

554-
const result = await handleRequestClearNonce(provider)
563+
const message: WebviewMessage = {
564+
type: "requestClearNonce",
565+
requestId: "req-nonce-2",
566+
}
567+
568+
await handleRequestClearNonce(provider, message)
555569

556-
expect(result).toBeNull()
570+
expect(provider.postMessageToWebview).toHaveBeenCalledWith({
571+
type: "requestClearNonceResponse",
572+
requestId: "req-nonce-2",
573+
clearNonce: null,
574+
error: expect.stringContaining("[STATS_HANDLER/clear/002]"),
575+
})
557576
})
558577
})
559578
})

src/core/webview/usageStatsMessageHandler.ts

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -322,20 +322,54 @@ export async function handleExportUsageStats(
322322
}
323323

324324
/**
325-
* Issues a clear confirmation nonce and returns it to the webview.
326-
* The webview must include this nonce in the subsequent `clearUsageStats` message.
325+
* Handles the `requestClearNonce` message (B2 fix).
327326
*
328-
* This is called from the webview's confirmation dialog flow.
329-
* The nonce is short-lived (5 minutes) and single-use.
327+
* Issues a host-generated clear confirmation nonce and posts it back to the
328+
* webview as `requestClearNonceResponse`. The webview must include this nonce
329+
* in the subsequent `clearUsageStats` message.
330+
*
331+
* Previously the webview generated its own nonce, which the host never stored,
332+
* so `clearStats` always failed with "nonce mismatch". The nonce is now
333+
* host-issued, short-lived (5 minutes), and single-use — matching the security
334+
* design intent.
330335
*/
331-
export async function handleRequestClearNonce(provider: ClineProvider): Promise<string | null> {
332-
const service = provider.getUsageStatsService()
336+
export async function handleRequestClearNonce(provider: ClineProvider, message: WebviewMessage): Promise<void> {
337+
const requestId = message.requestId
333338

334-
if (!service) {
335-
return null
336-
}
339+
try {
340+
const service = provider.getUsageStatsService()
341+
342+
if (!service) {
343+
await provider.postMessageToWebview({
344+
type: "requestClearNonceResponse",
345+
requestId,
346+
clearNonce: null,
347+
error: "[STATS_HANDLER/clear/002] Usage stats service is unavailable",
348+
})
349+
return
350+
}
351+
352+
const nonce = service.issueClearNonce()
337353

338-
return service.issueClearNonce()
354+
await provider.postMessageToWebview({
355+
type: "requestClearNonceResponse",
356+
requestId,
357+
clearNonce: nonce,
358+
})
359+
} catch (error) {
360+
const errorMessage = error instanceof Error ? error.message : String(error)
361+
362+
provider.log(
363+
`[STATS_HANDLER/clear/003] Error issuing clear nonce: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
364+
)
365+
366+
await provider.postMessageToWebview({
367+
type: "requestClearNonceResponse",
368+
requestId,
369+
clearNonce: null,
370+
error: `[STATS_HANDLER/clear/003] Failed to issue clear nonce: ${errorMessage}`,
371+
})
372+
}
339373
}
340374

341375
// Re-export StatsServiceError for convenience in tests

src/core/webview/webviewMessageHandler.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
handleGetUsageStats,
5454
handleClearUsageStats,
5555
handleExportUsageStats,
56+
handleRequestClearNonce,
5657
} from "./usageStatsMessageHandler"
5758
import { changeLanguage, t } from "../../i18n"
5859
import { Package } from "../../shared/package"
@@ -3948,6 +3949,11 @@ export const webviewMessageHandler = async (
39483949
break
39493950
}
39503951

3952+
case "requestClearNonce": {
3953+
await handleRequestClearNonce(provider, message)
3954+
break
3955+
}
3956+
39513957
case "exportUsageStats": {
39523958
await handleExportUsageStats(provider, message)
39533959
break

src/package.nls.ca.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/package.nls.de.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/package.nls.es.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/package.nls.fr.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)