[iscekic]

Why

A production report says the Kilo Platform MCP signs the user out several times per day. The session is a month long, so repeated sign-ins cannot be expiry. Investigation shows two causes are indistinguishable in production telemetry:

• kilo_mcp_oauth_sign_in_failed carries only the OAuth error code, so Client ID mismatch, Grant not found, Invalid refresh token and Refresh token has expired all look identical.
• The refresh-token reuse guard answers from the Durable Object before the provider runs, so its rejection never reaches the provider's onError hook and leaves no record at all.

That gap blocks both diagnosis and any future reply to a report like this one.

What

• onError maps the library error_description through a fixed 15-entry table to a detail slug and emits it. Unknown descriptions become other. The raw text never leaves the worker: the library interpolates into one of its descriptions.
• The reuse guard emits the same failed event with detail refresh_token_reuse and the account the destroyed grant belonged to. It never emits the grant id, the refresh token or the access token.
• Both use the existing kilo_mcp_oauth_sign_in_failed event, so no new event name or dashboard is needed.

Example line now produced by the real provider and Durable Object path:

[kilo-mcp] analytics kilo_mcp_oauth_sign_in_failed {"phase":"failed","reason":"invalid_grant","detail":"refresh_token_reuse",...,"userId":"..."}

Testing

• vitest run across refresh-reuse, provider-hooks, analytics, index and store/oauth-store: 187 tests pass.
• The Durable Object test harness (real provider plus DO SQLite) asserts that a replay of a rotated refresh token revokes the grant and emits the new event with the account attached.
• New cases cover the vocabulary table, the other fallback, the exact property allowlist, and the no-emission path when the guard forwards the request.

Risk

Telemetry only. The guard's decision and every token response are unchanged. One new property on an existing event.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental fix set is clean: the error-description lookup now requires an own property, and the telemetry detail field is a closed union shared by both emitters. I found no new problems in the five changed files.

Assumptions: [INFERENCE] The library's exact error_description strings for @cloudflare/workers-oauth-provider remain unverified because dependencies are not installed in this sandbox; unmapped or undefined descriptions fall through to other. Validation commands (vitest, tsc) could not be run in this read-only session, so the new Object.hasOwn path and the retyped table were reviewed statically against tsconfig.json (lib: ES2022).

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0

Files Reviewed (5 files)

• services/kilo-mcp/src/analytics.ts
• services/kilo-mcp/src/oauth/provider-hooks.ts
• services/kilo-mcp/src/oauth/provider-hooks.test.ts
• services/kilo-mcp/src/oauth/refresh-reuse.ts
• services/kilo-mcp/src/oauth/refresh-reuse.test.ts

Previous Review Summary (commit e1a0169)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e1a0169)

Status: 2 Issues Found | Recommendation: Merge (suggestions only)

Executive Summary

The telemetry-only change is sound; two hardening suggestions concern the new failure-detail vocabulary's prototype lookup and its unbounded string type.

Assumptions: [INFERENCE] The library's exact error_description strings for @cloudflare/workers-oauth-provider 0.10.3 could not be verified because dependencies are not installed in this sandbox; unmapped strings safely fall back to other. The PR body claims the DO harness asserts the reuse event, but no store/oauth-store test changes are in the diff, so the new analytics wiring in index.ts is covered only by unit tests with an injected fake.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
services/kilo-mcp/src/oauth/provider-hooks.ts	107	Prototype-inherited keys (constructor, toString) bypass the other fallback instead of returning a slug.
services/kilo-mcp/src/analytics.ts	107	detail?: string cannot enforce the documented fixed-vocabulary / no-free-text invariant.

Files Reviewed (7 files)

• services/kilo-mcp/src/analytics.ts - 1 suggestion
• services/kilo-mcp/src/analytics.test.ts
• services/kilo-mcp/src/index.ts
• services/kilo-mcp/src/oauth/provider-hooks.ts - 1 suggestion
• services/kilo-mcp/src/oauth/provider-hooks.test.ts
• services/kilo-mcp/src/oauth/refresh-reuse.ts
• services/kilo-mcp/src/oauth/refresh-reuse.test.ts

Fix these issues in Kilo Cloud

---

_{Reviewed by deepseek-flash-latest · Input: 29.3K · Output: 3.6K · Cached: 238.5K}

_{Review guidance: REVIEW.md from base branch main}