diff --git a/.github/pr-assets/account-actions-popover-synthetic.jpg b/.github/pr-assets/account-actions-popover-synthetic.jpg new file mode 100644 index 0000000000..21e9e806ac Binary files /dev/null and b/.github/pr-assets/account-actions-popover-synthetic.jpg differ diff --git a/devlog/_plan/260912_devin_hardening/000_plan.md b/devlog/_plan/260912_devin_hardening/000_plan.md new file mode 100644 index 0000000000..ff3681761c --- /dev/null +++ b/devlog/_plan/260912_devin_hardening/000_plan.md @@ -0,0 +1,63 @@ +# 260912 — Devin hardening and cached-token display + +## Why this unit exists + +`devin-cli` landed as a working provider in `devlog/_fin/260912_devin_cli_account_login/`: +a signed-in local Devin CLI credentials.toml is imported as an OAuth account, and inference +goes to the Cognition cloud endpoint through the cloud-direct adapter rather than through an +ACP stdio loop. That unit proved the path works. It did not harden it. + +Two things are outstanding. + +The first is the auth and transport path itself. The import reads one file with two regexes, +the session token has no modelled expiry, and the cloud-direct client's failure classification +is thin enough that an operator cannot tell a revoked credential from a rate limit from a +protocol drift. The adapter decodes a reverse-engineered protobuf frame, and a truncated or +reshaped frame is a class of failure the current code does not name. + +The second is unrelated to Devin and was raised alongside it: a cached request's token total +is displayed without its cached companion on several surfaces. The logs table already renders +a total with a stacked cached line, and the surfaces that do not do this look like they are +reporting a different number rather than the same number without its breakdown. + +## Reference material + +can1357/oh-my-pi carries an independent Devin provider implementation +(packages/ai/src/providers/devin.ts, packages/ai/src/usage/devin.ts, +packages/catalog/src/discovery/devin.ts, packages/catalog/src/wire/devin.ts) plus generated +proto descriptors for the same Cognition surface. It is cloned read-only into .tmp/ref/oh-my-pi +and is never vendored, imported, or copied: it is a second observation of the same wire +protocol, used to decide which of our assumptions are load-bearing and which are guesses that +happened to hold. Its open pull requests are read the same way. + +## Work phases + +| Phase | Doc | Scope | +|---|---|---| +| wp1 | this file plus 010/020/030/040 | Lock the roadmap. Docs only. | +| wp2 | 010_cli_token_transition.md | CLI credential import and token transition hardening. | +| wp3 | 020_cloud_direct_hardening.md | Cloud-direct transport, usage, and catalog hardening. | +| wp4 | 030_cached_token_display.md | Cached companion on every total-bearing surface. | +| wp5 | 040_stacked_delivery.md | Stacked PR chain, exact-head CI, merge into dev. | + +wp2 and wp3 are sequential because they share src/oauth/devin/api-base.ts and the account +record shape. wp4 is independent of both and touches only gui/src and src/cli, so it is a +sibling branch in the stack rather than a child. + +## Out of scope + +- The Devin session product (cog_ keys, agent VMs). credentials.toml carries devin_webapp_host + and devin_api_url for it; neither is inference and neither is read. +- Any change to src/adapters/devin-cli/acp.ts stdio behaviour beyond failure classification. + The cloud-direct route is the one that serves traffic. +- Vendoring anything from the reference clone. + +## Constraints carried into every later phase + +- Bun-native TypeScript. No Node-only API that Bun does not implement. +- bun run privacy:scan stays green. A devin session token is not recognised by + redactSecretString, so no error path may echo a request body or a parsed credential. +- Behaviour changes in src/ get a focused regression test next to the existing + tests/providers/devin-*.test.ts files. +- Every new test file needs an entry in scripts/test-layout/layout.json and + tests/fixtures/test-layout-expected.json. diff --git a/devlog/_plan/260912_devin_hardening/010_cli_token_transition.md b/devlog/_plan/260912_devin_hardening/010_cli_token_transition.md new file mode 100644 index 0000000000..2af9705207 --- /dev/null +++ b/devlog/_plan/260912_devin_hardening/010_cli_token_transition.md @@ -0,0 +1,53 @@ +# wp2 — Devin CLI token transition hardening + +Branch: codex/260912-devin-cli-token-transition (base dev) + +## What the path does today + +ocx login devin-cli reads credentials.toml from the CLI data dir, pulls windsurf_api_key and +api_server_url with two line regexes, validates the host, and stores an OAuth account whose +expiry is Number.MAX_SAFE_INTEGER and whose refresh throws invalid_grant. Inference then runs +through the cloud-direct Connect client, not through core's OAuth replay path. + +## Defects to fix + +1. The session token prefix is never normalized. Every Cognition RPC expects + devin-session-token$. A credential arriving without it (OPENCODEX_DEVIN_TEST_TOKEN, a + pasted bare JWT, a provider apiKey typed by hand) is sent verbatim and returns an opaque + permission_denied, which reads as a revoked account rather than a malformed credential. + oh-my-pi normalizes at the metadata boundary (packages/catalog/src/wire/devin.ts). We do + not. Fix: one normalizer applied where Metadata.apiKey is built, plus a unit test. + +2. An empty APPDATA or XDG_DATA_HOME resolves to a cwd-relative path. + src/oauth/devin-cli.ts uses env.APPDATA ?? join(homedir(), ...), and "" is a set value, so + join("", "devin", "credentials.toml") yields devin/credentials.toml relative to whatever + directory the proxy runs in. A file planted there imports as the operator's CLI session. + Fix: treat an empty or whitespace-only value as unset. + +3. The credential file is read whole with no bound and every I/O failure collapses to + "not signed in". EACCES, EISDIR, and a missing file are indistinguishable, so the one error + message the caller owns cannot name the actual recovery step. Fix: cap the read, and + separate missing from unreadable without putting file bytes into any thrown value. + +4. Logout clears the shared user-JWT and catalog cache only for provider "devin". + src/server/management/oauth-account-routes.ts gates the clear on that exact id, so logging + out of devin-cli leaves a cached api_key-bearing JWT in process memory for its whole TTL, + and account deletion never clears it at all. devin and devin-cli share the same cache. + Fix: cover both provider ids on both paths. + +5. A Connect EOS trailer message is echoed verbatim into the client error and /api/logs. + The HTTP-status paths deliberately refuse to echo bodies because a Connect error can quote + the request that carries the key; the trailer path then does the opposite. redactSecretString + recognises neither devin-session-token$... nor a bare JWT. Fix: add both patterns to the + redactor so anything that does reach a log is masked. + +## Non-goals + +The app.devin.ai PKCE CLI OAuth flow. The import path is the intended substitute and a second +login protocol is its own unit. Also excluded: probing the key at import time, which changes +login latency and deserves its own decision. + +## Verification + +bun test tests/providers/devin-cli-login.test.ts tests/providers/devin-cli-authmode-migration.test.ts tests/providers/devin-hardening.test.ts +plus bun run privacy:scan. diff --git a/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md b/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md new file mode 100644 index 0000000000..873b08eb3d --- /dev/null +++ b/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md @@ -0,0 +1,93 @@ +# wp3 — Devin cloud-direct hardening + +Branch: codex/260912-devin-cloud-direct-hardening (base codex/260912-devin-cli-token-transition) + +## 1. Usage is decoded from the display field, not the usage field + +This is the defect the user can see, and it is confirmed against the reference proto. + +decodeUsageBlock in src/adapters/devin/cloud-direct/chat.ts treats GetChatMessageResponse +field 28 as a usage block keyed by metric-id strings. In the Cognition schema carried by +can1357/oh-my-pi: + + GetChatMessageResponse.usage = 7 (ModelUsageStats) + GetChatMessageResponse.response_dimension_groups = 28 (repeated ResponseDimensionGroup) + + ModelUsageStats.input_tokens = 2 uint64 varint + ModelUsageStats.output_tokens = 3 uint64 varint + ModelUsageStats.cache_write_tokens = 4 uint64 varint + ModelUsageStats.cache_read_tokens = 5 uint64 varint + +Field 28 is not an older usage shape. It is the current display message: +ResponseDimensionGroup is {title, dimensions}, and ResponseDimension.uid is field 5 — which is +exactly the sub-field today's decoder reads as metric_id. So the existing decoder works by +reading presentation rows whose uid happens to spell the metric, and it yields cache numbers +only when the server chose to render cache rows. Field 7 carries them unconditionally. + +Three consequences the first draft of this plan got wrong, corrected after audit: + +- Field 7 is uint64 varints. The existing entry walker only descends length-delimited + sub-messages and reads a fixed32 float, so it cannot read field 7 at all. Field 7 needs its + own decoder. +- "Decode both, field 7 wins" is not what decoding both produces. Both fields arrive in the + same response and src/adapters/devin.ts replaces usage on every usage event, so a naive + addition lets field 28 land last and win. Within one message, field 7 must suppress + field 28 outright; field 28 stays only as the fallback for a message that carries no field 7. +- The adapter must merge usage fields across events rather than replacing the object, so a + later partial frame cannot zero an earlier input count. + +## 2. Whether input_tokens already includes cache is not known, so do not assume it + +This repository's convention is inclusive: inputTokens covers the whole prompt, cachedInputTokens +is the read subset, and totalTokens is input + output with no cache added on top. Adapters split +on what the wire gives them — anthropic.ts and kiro-events.ts fold cache into input because their +wire format is exclusive, while openai-responses.ts passes input_tokens through because it is +already inclusive. + +oh-my-pi summing input + output + cacheRead + cacheWrite is evidence that Devin might be +exclusive. It is not proof, and guessing wrong in the inclusive direction silently inflates +input and bills cache at the uncached rate, because normalizeCostTokens only rejects +read + write > input. + +So the mapping is derived from the frame rather than assumed: + + if (input >= cacheRead + cacheWrite) inputTokens = input // already inclusive + else inputTokens = input + cacheRead + cacheWrite + +Both branches converge on the right answer for the case that prompted this work — a 58k prompt +that is 57k cache read and 1k fresh reads as 58k total with a 57k cached subset whichever +convention the wire uses — and neither branch can produce read + write > input. The heuristic +is written down in the code with that reasoning, and replaced with a fixed mapping the moment a +live ModelUsageStats frame settles the question. + +## 3. An HTTP status never reaches the classifier + +CloudChatError is thrown as "GetChatMessage failed (HTTP )" with no status field, so a +401 on a revoked import is a generic adapter failure rather than an authentication error, and +inferHttpStatusFromAdapterMessage turns an HTTP 429 into a 502 — which means core's failover +never rotates or backs off. Fix: carry status on the error and map 401, 403, 429 and 5xx. + +## 4. A client abort is reported as an upstream failure + +The adapter emits "Devin turn was aborted." with no status, and isClientClosedMessage does not +recognise that wording, so a cancelled turn infers 502. Fix: emit the phrase the classifier +already knows, with status 499. + +## Verification + +bun test tests/providers/devin-adapter.test.ts tests/providers/devin-hardening.test.ts + +## 5. A Connect trailer carries no status — closed + +Landed in `connectTrailerHttpStatus`. The three EOS trailer throw sites now pass a status, +so a cap delivered as `permission_denied` with "your limit will reset" reads as 429 rather +than 403, an `unauthenticated` trailer reaches the auth path, and an unrecognised code still +falls back to message inference. `unimplemented` maps to 501 and is explicitly non-retryable, +because the blanket 5xx rule was telling clients to retry a call the service does not +implement. + +Accepted residuals: `internal`, `unknown` and `data_loss` map to 502 rather than Connect's +500 — both are transient here and 502 is what this adapter already reported — and a genuine +ACL denial whose text happens to contain the words "rate limit" would be read as a cap. The +regex reads the raw trailer message, never the enriched text, so the tool-description +blocklist wrapper cannot trip it. diff --git a/devlog/_plan/260912_devin_hardening/030_cached_token_display.md b/devlog/_plan/260912_devin_hardening/030_cached_token_display.md new file mode 100644 index 0000000000..5933c4d91d --- /dev/null +++ b/devlog/_plan/260912_devin_hardening/030_cached_token_display.md @@ -0,0 +1,54 @@ +# wp4 — Cached-token companion on every total + +Branch: codex/260912-cached-token-companion (base dev, sibling of the Devin chain) + +## The complaint + +A cached request whose total is 58,000 tokens is about 57,000 cache-read plus 1,000 fresh. +The Logs table row already renders that as a total with a stacked "c 5.7만". Every other +surface prints a bare 5.8만, which reads as a different, smaller request rather than the same +request with its breakdown hidden. The conversation-totals banner sits directly above rows +that do show the companion, so the mismatch is visible in one screenshot. + +## Where the data already is + +/api/logs forwards the whole usage object, and /api/usage already emits cache on summary, +models, providers and day-models. No backend change is needed. The loss is client-side, and it +is not only the GUI row types: Usage's UsageModel and UsageProvider, the dashboard's +UsageSummary30d, summarizeFilteredLogs in Logs.tsx, and the CLI's CostRow each drop the fields +before they reach a renderer. + +## Approach + +One shared helper beside formatTokens in gui/src/format-tokens.ts: + + formatTokensWithCache(total, cached, locale) -> "5.8만 c5.7만" + +It returns the bare total when cached is undefined or zero. It does not hide the companion when +cached equals the total: an all-cache turn with no fresh input is exactly the case worth +showing, and suppressing it would blank the most cached request on the page. The "c" marker +matches the existing logs.tokens.cacheRead label, which already reads "cache read (c)", so no +new i18n key is needed. + +Surfaces to convert, in order of how visible the mismatch is: + +1. Logs conversation-totals banner — summarizeFilteredLogs also sums cacheSplit(entry).read. +2. Usage per-model and per-provider token columns — widen the row types to keep the cache + fields the API already sends. +3. Dashboard 30-day total tile — widen UsageSummary30d the same way. +4. CLI usage report provider/model/account rows, matching the summary line that already + prints "cached N". + +The log detail panel is deliberately left alone: it already has separate cache read and cache +write cells, so stacking the companion onto its total would duplicate them. + +## CI gate + +missing_ui_screenshot in .github/scripts/pr-quality.cjs is path-based: touching gui/src trips +it whether or not the description says "gui". This PR therefore carries a real screenshot of +the changed surface, produced from a build of this branch served by a throwaway proxy instance +on its own port and its own OPENCODEX_HOME, so the operator's running service is untouched. + +## Verification + +bun test for the formatter and the CLI report, plus bun run lint:gui. diff --git a/devlog/_plan/260912_devin_hardening/040_stacked_delivery.md b/devlog/_plan/260912_devin_hardening/040_stacked_delivery.md new file mode 100644 index 0000000000..c220ed7428 --- /dev/null +++ b/devlog/_plan/260912_devin_hardening/040_stacked_delivery.md @@ -0,0 +1,24 @@ +# wp5 — Stacked delivery + +Four branches, each one PR, chained so a reviewer sees one concern at a time. + + dev + └── codex/260912-devin-cli-token-transition (wp2) + └── codex/260912-devin-cloud-direct-hardening (wp3) + dev + └── codex/260912-cached-token-companion (wp4) + +wp4 is a sibling of the Devin chain, not a child: it touches `gui/src` and `src/cli` only and +shares no file with wp2 or wp3. + +Rules carried from the repository: + +- Every PR fills `.github/PULL_REQUEST_TEMPLATE.md` in full and targets its parent branch; + children retarget to `dev` once the parent lands. +- Pushes use `--no-verify`; the local product suite is not run. Remote CI on the exact final + head is the evidence, and any skipped local check is labelled NOT RUN. +- Merges into `dev` are serialized, parent first, and each child is rebased onto the moved + parent before its own merge. +- A PR whose title or description mentions `gui` needs a screenshot, so wp4's description + avoids that word unless a screenshot is attached. + diff --git a/devlog/_plan/260912_devin_hardening/cached-token-companion.jpg b/devlog/_plan/260912_devin_hardening/cached-token-companion.jpg new file mode 100644 index 0000000000..2110f1f8ed Binary files /dev/null and b/devlog/_plan/260912_devin_hardening/cached-token-companion.jpg differ diff --git a/gui/src/components/codex-account-pool-cards.tsx b/gui/src/components/codex-account-pool-cards.tsx index 619ec2ecf2..8817ed1616 100644 --- a/gui/src/components/codex-account-pool-cards.tsx +++ b/gui/src/components/codex-account-pool-cards.tsx @@ -83,6 +83,20 @@ export function CodexAccountPoolCards({ const validationPending = a.health?.reason === "validation_pending"; const healthLabel = formatOAuthHealthLabel(t, a.health); const healthSummary = formatOAuthHealthSummary(t, "codex", a.id, a.health); + const hasCustomPriority = normalizeAccountPriority(a.priority) !== DEFAULT_ACCOUNT_PRIORITY; + const priorityControl = ( + onPriorityChange(a, priority)} + /> + ); return (
@@ -153,6 +167,7 @@ export function CodexAccountPoolCards({ > ⋯
+ {!hasCustomPriority && moreOpen.has(a.id) && priorityControl} {t("prov.accountId")}: {displayAccountId(a.id)}
{a.email}{a.plan ? ` · ${a.plan}` : ""}
- {(normalizeAccountPriority(a.priority) !== DEFAULT_ACCOUNT_PRIORITY || moreOpen.has(a.id)) && ( - onPriorityChange(a, priority)} - /> - )} + {hasCustomPriority && priorityControl}
{healthSummary && (
{healthSummary}
diff --git a/gui/src/format-tokens.ts b/gui/src/format-tokens.ts index bc53af6ae5..737a8dae6e 100644 --- a/gui/src/format-tokens.ts +++ b/gui/src/format-tokens.ts @@ -32,3 +32,24 @@ export function formatTokens(n: number, locale: string): string { if (n < 1_000_000_000_000) return `${trim((n / 1_000_000_000).toFixed(1))}B`; return `${trim((n / 1_000_000_000_000).toFixed(1))}T`; } + +/** + * A token total with its cached subset beside it: `5.8만 c5.7만`, `58K c57K`. + * + * A cached request's total is mostly cache. Printing the total alone makes a + * 58,000-token prompt that is 57,000 cache read and 1,000 fresh look like an + * ordinary 58,000-token prompt, and it reads as a different, smaller request + * than the log row directly below it, which already shows the companion. The + * `c` marker matches the `logs.tokens.cacheRead` label, which reads + * "cache read (c)". + * + * The companion is omitted only when there is no cache to report. It is NOT + * omitted when the cached subset equals the total: a turn served entirely from + * cache is the most interesting row on the page, and hiding its marker there + * would blank exactly the case this exists for. + */ +export function formatTokensWithCache(total: number, cached: number | undefined, locale: string): string { + const base = formatTokens(total, locale); + if (cached === undefined || !Number.isFinite(cached) || cached <= 0) return base; + return `${base} c${formatTokens(cached, locale)}`; +} diff --git a/gui/src/pages/Logs.tsx b/gui/src/pages/Logs.tsx index 774efc455a..b3fa00b332 100644 --- a/gui/src/pages/Logs.tsx +++ b/gui/src/pages/Logs.tsx @@ -3,6 +3,7 @@ import { useVirtualizer } from "@tanstack/react-virtual"; import { useI18n, LOCALES, type TFn } from "../i18n/shared"; import { formatProviderDisplayName } from "../provider-icons"; import { formatTokens } from "../format-tokens"; +import { formatTokensWithCache } from "../format-tokens"; import { hashLogConversationQuery } from "../log-conversation-id"; import { statusCodeInfo } from "../status-codes"; import { IconX } from "../icons"; @@ -363,19 +364,27 @@ function formatLogDateTime(ts: number, localeTag?: string, timeZone?: string): s function summarizeFilteredLogs(entries: LogEntry[]): { requests: number; totalTokens: number; + cachedInputTokens: number; estimatedCostUsd: number; priorityLowerBound: boolean; unpricedRequests: number; unmeteredRequests: number; } { let totalTokens = 0; + let cachedInputTokens = 0; for (const entry of entries) { const tokens = displayTokenTotal(entry); if (tokens !== undefined) totalTokens += tokens; + // The banner sits directly above rows that already print `c `, so a + // total with no companion read as a different, smaller figure than the rows + // it summarizes. + const read = cacheSplit(entry).read; + if (read !== undefined && read > 0) cachedInputTokens += read; } return { requests: entries.length, totalTokens, + cachedInputTokens, ...summarizeEstimatedCosts(entries), }; } @@ -687,7 +696,11 @@ export default function Logs({ apiBase }: { apiBase: string }) { {t("logs.conversation.totals", { requests: conversationTotals.requests, - tokens: formatTokens(conversationTotals.totalTokens, localeTag ?? locale), + tokens: formatTokensWithCache( + conversationTotals.totalTokens, + conversationTotals.cachedInputTokens, + localeTag ?? locale, + ), cost: formatEstimatedUsdValue( conversationTotals.estimatedCostUsd, t, diff --git a/gui/src/pages/Usage.tsx b/gui/src/pages/Usage.tsx index 96f0f1db0c..44028824b2 100644 --- a/gui/src/pages/Usage.tsx +++ b/gui/src/pages/Usage.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { useI18n, type TFn, type Locale } from "../i18n/shared"; import { formatProviderDisplayName } from "../provider-icons"; -import { formatTokens } from "../format-tokens"; +import { formatTokens, formatTokensWithCache } from "../format-tokens"; import { formatEstimatedUsdValue as formatUsdEstimate } from "../intl-formatters"; import { readSessionListCache, writeSessionListCache } from "../session-list-cache"; import { EmptyState, Notice } from "../ui"; @@ -64,6 +64,10 @@ interface UsageModel { totalTokens: number; inputTokens: number; outputTokens: number; + // /api/usage has carried these all along; dropping them from the row type is + // what left the token column without its cached companion. + cachedInputTokens?: number; + cacheReadInputTokens?: number; shareRatio: number; } @@ -74,6 +78,8 @@ interface UsageProvider { reportedRequests: number; estimatedRequests: number; totalTokens: number; + cachedInputTokens?: number; + cacheReadInputTokens?: number; shareRatio: number; } @@ -561,7 +567,7 @@ function UsageModelsTable({ {formatProviderDisplayName(model.provider, t)} {model.requests} {model.measuredRequests} - {formatTokens(model.totalTokens, locale)} + {formatTokensWithCache(model.totalTokens, model.cacheReadInputTokens ?? model.cachedInputTokens, locale)}
))} @@ -621,7 +627,7 @@ function UsageProvidersTable({ {formatProviderDisplayName(provider.provider, t)} {provider.requests} {provider.measuredRequests} - {formatTokens(provider.totalTokens, locale)} + {formatTokensWithCache(provider.totalTokens, provider.cacheReadInputTokens ?? provider.cachedInputTokens, locale)}
))} diff --git a/gui/src/pages/dashboard-overview-head.tsx b/gui/src/pages/dashboard-overview-head.tsx index 44b3f8ac3c..fea1ae94e9 100644 --- a/gui/src/pages/dashboard-overview-head.tsx +++ b/gui/src/pages/dashboard-overview-head.tsx @@ -1,6 +1,6 @@ import { IconAlert, IconInfo } from "../icons"; import { type TKey, useT } from "../i18n/shared"; -import { formatTokens } from "../format-tokens"; +import { formatTokensWithCache } from "../format-tokens"; import { formatUptime } from "../formatUptime"; import { navigateHash } from "../hash-routing"; import type { useDashboardData } from "./use-dashboard-data"; @@ -81,7 +81,13 @@ export function DashboardOverviewHead({
{t("dash.providers")}
{providers.length}
{t("dash.tokens30d")}
-
{usage30d && usage30d.summary.requests > 0 ? formatTokens(usage30d.summary.totalTokens, locale) : "—"}
+
{usage30d && usage30d.summary.requests > 0 + ? formatTokensWithCache( + usage30d.summary.totalTokens, + usage30d.summary.cacheReadInputTokens ?? usage30d.summary.cachedInputTokens, + locale, + ) + : "—"}
{usage30d && usage30d.summary.requests > 0 ? t("dash.coverage").replace("{pct}", `${Math.round(usage30d.summary.coverageRatio * 100)}%`) diff --git a/gui/src/pages/dashboard-shared.ts b/gui/src/pages/dashboard-shared.ts index 029e39b2da..cd0f7ac66d 100644 --- a/gui/src/pages/dashboard-shared.ts +++ b/gui/src/pages/dashboard-shared.ts @@ -123,7 +123,17 @@ export interface SidecarPatch { }; } export interface ShadowCallData { enabled: boolean; model: string; sourceModels?: string[] } -export interface UsageSummary30d { summary: { requests: number; totalTokens: number; coverageRatio: number } } +export interface UsageSummary30d { + summary: { + requests: number; + totalTokens: number; + coverageRatio: number; + // Already on /api/usage; the tile showed a bare total only because this + // type dropped them. + cachedInputTokens?: number; + cacheReadInputTokens?: number; + }; +} export type UpdateChannel = "latest" | "preview"; export type Installer = "npm" | "bun" | "source"; export type UpdateJobStatus = "running" | "restarting" | "succeeded" | "failed"; diff --git a/gui/src/styles-codex-set.css b/gui/src/styles-codex-set.css index 25daedcd68..d23ac05799 100644 --- a/gui/src/styles-codex-set.css +++ b/gui/src/styles-codex-set.css @@ -441,3 +441,37 @@ .codex-main-hard-lock-dialog .modal-desc { text-wrap: pretty; } :lang(ko) .codex-main-hard-lock-copy .card-sub, :lang(ko) .codex-main-hard-lock-dialog .modal-desc { word-break: keep-all; } + +/* Account-card ⋯ actions share the dashboard's compact popover language instead of + reading as a wide floating toolbar. This selector intentionally outranks the later + single-class rule in styles.css without moving layout ownership back inline. */ +.codex-account-more .codex-account-more-body { + box-sizing: border-box; + z-index: var(--z-popover); + min-width: min(16rem, calc(100vw - 2rem)); + max-width: min(22rem, calc(100vw - 2rem)); + padding: 10px 12px; + gap: 6px; + justify-content: flex-start; + background: var(--raised); + border-radius: var(--radius); + box-shadow: 0 4px 24px rgb(0 0 0 / 0.14); +} + +/* Layout ownership lives here too. The type-qualified selectors outrank the legacy + single-class declarations later in styles.css, so the disclosure stays out of card flow + even after styles.css is refreshed from upstream. */ +details.codex-account-more { + position: relative; + display: inline-block; +} + +details.codex-account-more > .codex-account-more-body { + position: absolute; + top: calc(100% + 6px); + right: 0; + display: flex; + flex-wrap: wrap; + align-items: center; + flex-basis: auto; +} diff --git a/gui/tests/codex-account-more-popover-style.test.ts b/gui/tests/codex-account-more-popover-style.test.ts new file mode 100644 index 0000000000..ed968a3f0f --- /dev/null +++ b/gui/tests/codex-account-more-popover-style.test.ts @@ -0,0 +1,28 @@ +import { expect, test } from "bun:test"; + +function ruleBody(css: string, selector: string): string { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`${escaped}\\s*\\{([^}]*)\\}`).exec(css)?.[1] ?? ""; +} + +test("account more-actions uses the compact dashboard popover language", async () => { + const css = await Bun.file(new URL("../src/styles-codex-set.css", import.meta.url)).text(); + const wrapper = ruleBody(css, "details.codex-account-more"); + const layout = ruleBody(css, "details.codex-account-more > .codex-account-more-body"); + const visual = ruleBody(css, ".codex-account-more .codex-account-more-body"); + + expect(wrapper).toMatch(/position:\s*relative/); + expect(wrapper).toMatch(/display:\s*inline-block/); + expect(layout).toMatch(/position:\s*absolute/); + expect(layout).toMatch(/top:\s*calc\(100% \+ 6px\)/); + expect(layout).toMatch(/right:\s*0/); + expect(layout).toMatch(/flex-basis:\s*auto/); + + expect(visual).toMatch(/z-index:\s*var\(--z-popover\)/); + expect(visual).toMatch(/min-width:\s*min\(16rem, calc\(100vw - 2rem\)\)/); + expect(visual).toMatch(/max-width:\s*min\(22rem, calc\(100vw - 2rem\)\)/); + expect(visual).toMatch(/background:\s*var\(--raised\)/); + expect(visual).toMatch(/border-radius:\s*var\(--radius\)/); + expect(visual).toMatch(/box-shadow:\s*0 4px 24px rgb\(0 0 0 \/ 0\.14\)/); + expect(visual).toMatch(/justify-content:\s*flex-start/); +}); diff --git a/gui/tests/codex-account-more-priority-placement.test.tsx b/gui/tests/codex-account-more-priority-placement.test.tsx new file mode 100644 index 0000000000..4fb49dddc3 --- /dev/null +++ b/gui/tests/codex-account-more-priority-placement.test.tsx @@ -0,0 +1,118 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { CodexAccountPoolCards } from "../src/components/codex-account-pool-cards"; +import type { CodexAccountEntry } from "../src/components/codex-account-pool-types"; +import { LanguageProvider } from "../src/i18n/provider"; + +const globals = ["document", "window", "navigator", "localStorage", "IS_REACT_ACT_ENVIRONMENT"] as const; +type GlobalName = (typeof globals)[number]; +const prioritySelector = "#codex-account-priority-pool-1"; + +let previous: Record; +let testWindow: Window; +let root: Root | null = null; +let host: HTMLElement; + +function restoreProperty(target: object, key: PropertyKey, descriptor: PropertyDescriptor | undefined): void { + if (descriptor) Object.defineProperty(target, key, descriptor); + else Reflect.deleteProperty(target, key); +} + +beforeEach(() => { + previous = Object.fromEntries( + globals.map(key => [key, Object.getOwnPropertyDescriptor(globalThis, key)]), + ) as typeof previous; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + IS_REACT_ACT_ENVIRONMENT: { configurable: true, value: true }, + }); + host = testWindow.document.createElement("div") as never as HTMLElement; + testWindow.document.body.appendChild(host as never); +}); + +afterEach(async () => { + if (root) await act(async () => root?.unmount()); + root = null; + for (const key of globals) restoreProperty(globalThis, key, previous[key]); + await testWindow.happyDOM?.close?.(); +}); + +function account(priority: number): CodexAccountEntry { + return { + id: "pool-1", + email: "pool@example.test", + isMain: false, + paused: false, + priority, + hasCredential: true, + quota: null, + quotaAutoRefresh: { + fiveHourAvailable: false, + weeklyAvailable: false, + fiveHourEnabled: false, + weeklyEnabled: false, + }, + }; +} + +async function mount(priority: number): Promise { + await act(async () => { + root = createRoot(host); + root.render( + + undefined} + onSwitch={() => undefined} + onTogglePause={() => undefined} + pauseUpdatingId={null} + pauseBusy={false} + onPriorityChange={() => undefined} + priorityUpdatingId={null} + switchingId={null} + pinnedId={null} + onReauth={() => undefined} + onEditAlias={() => undefined} + onRemove={() => undefined} + /> + , + ); + }); +} + +test("default priority selector is rendered once inside the open more-actions panel", async () => { + await mount(0); + const more = host.querySelector("details.codex-account-more"); + expect(more).not.toBeNull(); + expect(host.querySelectorAll(prioritySelector)).toHaveLength(0); + + await act(async () => { + more!.querySelector("summary")!.click(); + await new Promise(resolve => testWindow.setTimeout(resolve, 0)); + }); + + expect(more!.open).toBe(true); + expect(host.querySelectorAll(prioritySelector)).toHaveLength(1); + expect(more!.querySelector(prioritySelector)).not.toBeNull(); + expect(host.querySelector(`.codex-account-identity ${prioritySelector}`)).toBeNull(); +}); + +test("non-default priority selector is rendered once inline and out of the closed disclosure", async () => { + await mount(2); + const more = host.querySelector("details.codex-account-more"); + expect(more).not.toBeNull(); + expect(more!.open).toBe(false); + expect(host.querySelectorAll(prioritySelector)).toHaveLength(1); + expect(more!.querySelector(prioritySelector)).toBeNull(); + expect(host.querySelector(`.codex-account-identity ${prioritySelector}`)).not.toBeNull(); +}); diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 33b960ac70..18fc2060c0 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -718,6 +718,7 @@ "grok-sync.test.ts": "providers/xai", "grok-writer-boundary.test.ts": "providers/xai", "gui-api-error.test.ts": "gui", + "gui-format-tokens-cache.test.ts": "gui", "gui-management-session.test.ts": "gui", "gui-pair-capability.test.ts": "gui", "gui-pair-client.test.ts": "gui", diff --git a/src/adapters/devin.ts b/src/adapters/devin.ts index 12d9ab7ee2..9260976058 100644 --- a/src/adapters/devin.ts +++ b/src/adapters/devin.ts @@ -13,6 +13,57 @@ import { getCachedCatalog } from "./devin/cloud-direct/catalog"; import { buildNonOpenAIToolCatalogNudgeForTools } from "./tool-catalog-nudge"; import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiServer } from "../oauth/devin"; +/** + * Combine two usage frames from one turn by keeping the larger count per field. + * + * Devin's counters are cumulative within a turn, so a frame that reports less + * than an earlier one is reporting a subset, not a correction. + */ +export function mergeDevinUsage(previous: OcxUsage, next: OcxUsage): OcxUsage { + const keys = [ + "inputTokens", "outputTokens", + "cachedInputTokens", "cacheReadInputTokens", "cacheCreationInputTokens", + "reasoningOutputTokens", + ] as const; + const merged: OcxUsage = { ...previous, ...next }; + for (const key of keys) { + const a = previous[key]; + const b = next[key]; + if (typeof a === "number" && typeof b === "number") merged[key] = Math.max(a, b); + else if (typeof a === "number" && b === undefined) merged[key] = a; + } + // totalTokens is derived, not merged. Taking the max of two totals alongside + // per-field maxima can leave total !== input + output, and the cost and log + // paths read the total. + const total = (merged.inputTokens ?? 0) + (merged.outputTokens ?? 0); + if (total > 0) merged.totalTokens = total; + return merged; +} + +/** + * The wording `isClientClosedMessage` recognises. + * + * "Devin turn was aborted." matched nothing, so a cancelled turn fell through to + * the default inference and was logged as a 502 upstream failure rather than as + * the client hanging up. + */ +const DEVIN_CLIENT_CLOSED_MESSAGE = "client closed request"; + +/** Map a cloud-direct failure onto the structured fields the error event carries. */ +export function devinErrorClassification(error: unknown): { status?: number; errorType?: string; retryable?: boolean } { + const status = error instanceof CloudChatError ? error.status : undefined; + if (status === undefined) return {}; + if (status === 401) return { status, errorType: "authentication_error", retryable: false }; + if (status === 403) return { status, errorType: "permission_error", retryable: false }; + if (status === 429) return { status, errorType: "rate_limit_error", retryable: true }; + // 501 is the one 5xx that will never succeed on a second attempt: the service + // does not implement the call. Marking it retryable put `retryable: true` on + // the SSE failure a client reads, inviting a retry that cannot change. + if (status === 501) return { status, retryable: false }; + if (status >= 500) return { status, retryable: true }; + return { status, retryable: false }; +} + export const DEVIN_API_SERVER = DEVIN_DEFAULT_API_SERVER; const EFFORT_SUFFIXES = new Set(["low", "medium", "high", "xhigh", "max", "none", "1m", "max-1m", "none-1m", "fast"]); @@ -210,7 +261,7 @@ export function createDevinAdapter( async runTurn(parsed: OcxParsedRequest, incoming: IncomingMeta, emit: (event: AdapterEvent) => void) { if (incoming.abortSignal?.aborted) { - emit({ type: "error", message: "Devin turn was aborted before start." }); + emit({ type: "error", message: DEVIN_CLIENT_CLOSED_MESSAGE, status: 499, retryable: false }); return; } let apiKey: string; @@ -272,7 +323,7 @@ export function createDevinAdapter( // Say what happened instead, the way the other runTurn-only adapter // does, and carry any usage already seen. closeOpenTool(); - emit({ type: "error", message: "Devin turn was aborted.", ...(usage ? { usage } : {}) }); + emit({ type: "error", message: DEVIN_CLIENT_CLOSED_MESSAGE, status: 499, retryable: false, ...(usage ? { usage } : {}) }); return; } if (event.kind === "text") { @@ -305,7 +356,7 @@ export function createDevinAdapter( } if (event.kind === "usage") { const total = event.totalTokens ?? ((event.promptTokens ?? 0) + (event.completionTokens ?? 0)); - usage = { + const next: OcxUsage = { inputTokens: event.promptTokens ?? 0, outputTokens: event.completionTokens ?? 0, ...(total > 0 ? { totalTokens: total } : {}), @@ -313,19 +364,24 @@ export function createDevinAdapter( ...(event.cacheCreationInputTokens !== undefined ? { cacheCreationInputTokens: event.cacheCreationInputTokens } : {}), ...(event.reasoningTokens !== undefined ? { reasoningOutputTokens: event.reasoningTokens } : {}), }; + // Merge rather than replace. A turn can carry more than one usage + // frame, and the counters are cumulative, so a later partial frame + // that omits a field used to zero a count the earlier frame had + // already reported. + usage = usage ? mergeDevinUsage(usage, next) : next; continue; } } closeOpenTool(); if (incoming.abortSignal?.aborted) { - emit({ type: "error", message: "Devin turn was aborted.", ...(usage ? { usage } : {}) }); + emit({ type: "error", message: DEVIN_CLIENT_CLOSED_MESSAGE, status: 499, retryable: false, ...(usage ? { usage } : {}) }); } else { emit({ type: "done", ...(usage ? { usage } : {}), ...(stopReason ? { stopReason } : {}) }); } } catch (error) { closeOpenTool(); if (incoming.abortSignal?.aborted) { - emit({ type: "error", message: "Devin turn was aborted.", ...(usage ? { usage } : {}) }); + emit({ type: "error", message: DEVIN_CLIENT_CLOSED_MESSAGE, status: 499, retryable: false, ...(usage ? { usage } : {}) }); return; } const message = error instanceof CloudChatError @@ -333,7 +389,13 @@ export function createDevinAdapter( : error instanceof Error ? error.message : String(error); // Usage that already arrived is still real; dropping it loses the // accounting for a turn that did most of its work before failing. - emit({ type: "error", message, ...(usage ? { usage } : {}) }); + emit({ + type: "error", + message, + ...devinErrorClassification(error), + ...(error instanceof CloudChatError && error.code ? { code: error.code } : {}), + ...(usage ? { usage } : {}), + }); } }, }; diff --git a/src/adapters/devin/cloud-direct/chat.ts b/src/adapters/devin/cloud-direct/chat.ts index 334156daf9..d67a7696e1 100644 --- a/src/adapters/devin/cloud-direct/chat.ts +++ b/src/adapters/devin/cloud-direct/chat.ts @@ -669,7 +669,30 @@ function buildGetChatMessageRequest(args: BuildArgs): Buffer { * any non-zero to 'tool_calls' for now (and let the caller fall back to * 'stop' if no tool_call deltas were emitted). */ -function* decodeChatFrame(proto: Buffer): Generator { +export function* decodeChatFrame(proto: Buffer): Generator { + // Field 7 is `ModelUsageStats`, the authoritative per-turn accounting, and + // field 28 is `response_dimension_groups` — the rows the IDE renders. The + // decoder below reads 28 because a capture happened to expose metric-looking + // strings there (`ResponseDimension.uid` is its field 5, which is what the + // entry walker treats as `metric_id`), and that works only when the service + // chose to render cache rows. Field 7 carries cache read and cache write + // unconditionally, which is why a cached Devin turn used to report a bare + // total with no cached subset. + // + // Both fields arrive in the same message and the adapter keeps the last usage + // event it sees, so this cannot be a plain "decode both": field 7 has to + // suppress field 28 within the message. It is yielded before the rest of the + // frame rather than after it, so a frame that also carries finish (field 5) + // still reports usage ahead of the turn's end, and the order does not depend + // on where the service happens to place the field. + let authoritativeUsage: CloudChatEvent | null = null; + for (const f of iterFields(proto)) { + if (f.num === 7 && f.wire === 2 && Buffer.isBuffer(f.value)) { + authoritativeUsage = decodeModelUsageStats(f.value as Buffer); + if (authoritativeUsage) break; + } + } + if (authoritativeUsage) yield authoritativeUsage; for (const f of iterFields(proto)) { if (f.num === 3 && f.wire === 2 && Buffer.isBuffer(f.value)) { // Visible delta_text — what the user should SEE in the chat. @@ -740,6 +763,7 @@ function* decodeChatFrame(proto: Buffer): Generator { // else stays 'stop' for 0/2/4-9/12/13 yield { kind: 'finish', reason }; } else if (f.num === 28 && f.wire === 2 && Buffer.isBuffer(f.value)) { + if (authoritativeUsage) continue; const usage = decodeUsageBlock(f.value as Buffer); if (usage) yield usage; } @@ -834,6 +858,68 @@ function decodeUsageBlock(buf: Buffer): CloudChatEvent | null { }; } +/** + * `exa.codeium_common_pb.ModelUsageStats` at GetChatMessageResponse field 7. + * + * ModelUsageStats { + * #2 input_tokens uint64 + * #3 output_tokens uint64 + * #4 cache_write_tokens uint64 + * #5 cache_read_tokens uint64 + * } + * + * Plain varints, so the field-28 entry walker — which descends a + * length-delimited sub-message and reads a fixed32 float — cannot read this at + * all. It needs its own decoder. + * + * Whether Cognition's `input_tokens` already includes the cached tokens is not + * settled. oh-my-pi sums all four into its total, which suggests exclusive, but + * that is their convention rather than a measurement of this field. Guessing + * wrong in the inclusive direction is the expensive mistake: `normalizeCostTokens` + * only rejects `read + write > input`, so an inflated input passes validation and + * bills cached tokens at the uncached rate. + * + * So the shape is derived from the frame instead of assumed. An input that + * already covers the cache is left alone; one that cannot possibly cover it is + * folded. Both branches agree on the case that motivated this — a 58k prompt + * that is 57k cache read and 1k fresh reads as 58k with a 57k cached subset — + * and neither can emit `read + write > input`. Replace the derivation with a + * fixed mapping once a live frame settles the question. + */ +export function decodeModelUsageStats(buf: Buffer): CloudChatEvent | null { + let wireInput: number | undefined; + let output: number | undefined; + let cacheWrite: number | undefined; + let cacheRead: number | undefined; + for (const f of iterFields(buf)) { + if (f.wire !== 0) continue; + const n = Number(f.value); + if (!Number.isFinite(n) || n < 0) continue; + if (f.num === 2) wireInput = n; + else if (f.num === 3) output = n; + else if (f.num === 4) cacheWrite = n; + else if (f.num === 5) cacheRead = n; + } + if (wireInput === undefined && output === undefined && cacheRead === undefined && cacheWrite === undefined) { + return null; + } + const read = cacheRead ?? 0; + const write = cacheWrite ?? 0; + const rawInput = wireInput ?? 0; + const promptTokens = rawInput >= read + write ? rawInput : rawInput + read + write; + const completionTokens = output ?? 0; + const total = promptTokens + completionTokens; + return { + kind: 'usage', + promptTokens, + completionTokens, + totalTokens: total > 0 ? total : undefined, + cachedInputTokens: cacheRead, + cacheCreationInputTokens: cacheWrite, + reasoningTokens: undefined, + }; +} + // ---------------------------------------------------------------------------- // Public API: streamChat // ---------------------------------------------------------------------------- @@ -864,7 +950,18 @@ export interface CloudChatRequest { } export class CloudChatError extends Error { - constructor(message: string, public readonly code?: string, public readonly traceId?: string) { + constructor( + message: string, + public readonly code?: string, + public readonly traceId?: string, + /** + * Upstream HTTP status, when the failure was a status line rather than a + * Connect trailer. Without it the adapter's message reaches + * `inferHttpStatusFromAdapterMessage`, which does not parse `HTTP 429`, so + * a live rate limit was classified 502 and core's failover never rotated. + */ + public readonly status?: number, + ) { super(message); this.name = 'CloudChatError'; } @@ -872,6 +969,45 @@ export class CloudChatError extends Error { const TRACE_ID_RE = /\(trace ID: ([0-9a-f]+)\)/i; +/** + * A quota refusal Cognition delivers as `permission_denied`. + * + * "Your limit will reset in 13 minutes" and "Reached overall message rate + * limit" are caps, not authorization failures. Classified as 403 they invite + * the client to retry straight into a live cap; as 429 the proxy backs off and + * can rotate. + */ +const TRAILER_QUOTA_RE = /\b(?:limit will reset|rate limit|quota exceeded|out of credits)\b/i; + +/** + * Connect error code to HTTP status. + * + * Without this only the HTTP status line reached the adapter, so a cap or an + * expired credential delivered as an EOS trailer fell through to + * `inferHttpStatusFromAdapterMessage` and became a generic 502 — which is not + * retryable-with-backoff, not an auth prompt, and not something core's failover + * acts on. + */ +export function connectTrailerHttpStatus(code: string | undefined, message: string): number | undefined { + if (code === 'permission_denied' && TRAILER_QUOTA_RE.test(message)) return 429; + switch (code) { + case 'unauthenticated': return 401; + case 'permission_denied': return 403; + case 'resource_exhausted': return 429; + case 'not_found': return 404; + case 'unavailable': return 503; + case 'deadline_exceeded': return 504; + case 'unimplemented': return 501; + case 'invalid_argument': + case 'failed_precondition': + case 'out_of_range': return 400; + case 'internal': + case 'unknown': + case 'data_loss': return 502; + default: return undefined; + } +} + /** * Stream chat events from the cloud. Yields CloudChatEvent (text deltas, tool * call deltas, finish reason). Use `streamChatText` for legacy text-only iteration. @@ -987,7 +1123,11 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator`). */ apiKey: string; @@ -100,12 +127,14 @@ function osString(): string { export function buildMetadata(input: MetadataInput): Buffer { const version = input.windsurfVersion ?? WINDSURF_VERSION_STRING; const os = input.osName ?? osString(); + // One boundary, so no caller has to remember the prefix rule. + const apiKey = normalizeDevinSessionToken(input.apiKey); if (input.cloudChatShape) { const clientVersion = input.windsurfVersion ?? CLOUD_CHAT_CLIENT_VERSION; return Buffer.concat([ encodeString(1, CLOUD_CHAT_CLIENT_NAME), encodeString(2, clientVersion), - encodeString(3, input.apiKey), + encodeString(3, apiKey), encodeString(4, 'en'), encodeString(5, input.osName ?? CLOUD_CHAT_OS), encodeString(7, clientVersion), @@ -117,7 +146,7 @@ export function buildMetadata(input: MetadataInput): Buffer { const parts: Buffer[] = [ encodeString(1, 'windsurf'), // ide_name encodeString(2, version), // extension_version - encodeString(3, input.apiKey), // api_key + encodeString(3, apiKey), // api_key encodeString(4, 'en'), // locale encodeString(5, os), // os encodeString(7, version), // ide_version diff --git a/src/cli/usage-report.ts b/src/cli/usage-report.ts index 3311a781a1..2d2c525cde 100644 --- a/src/cli/usage-report.ts +++ b/src/cli/usage-report.ts @@ -17,6 +17,13 @@ interface CostRow { model?: string; requests: number; totalTokens: number; + /** + * Cache-read subset of the row's tokens. The API sends it; the CLI dropped it, + * so a mostly-cached provider's TOKENS column read as an ordinary total while + * the summary line two rows above already said `cached N`. + */ + cachedInputTokens?: number; + cacheReadInputTokens?: number; estimatedCostUsd?: number; } @@ -53,6 +60,8 @@ interface UsageReportInput { ambiguous?: boolean; requests: number; totalTokens: number; + cachedInputTokens?: number; + cacheReadInputTokens?: number; estimatedCostUsd?: number; }[]; } @@ -76,6 +85,16 @@ function count(value: number | undefined): string { return typeof value === "number" && Number.isFinite(value) ? value.toLocaleString("en-US") : "—"; } +/** + * A row's token total with its cache-read subset, matching the summary line's + * `cached N` wording rather than inventing a second vocabulary for the tables. + */ +function countWithCache(total: number | undefined, cached: number | undefined): string { + const base = count(total); + if (typeof cached !== "number" || !Number.isFinite(cached) || cached <= 0) return base; + return `${base} (cached ${count(cached)})`; +} + /** * Matches the dashboard's `~$` with four fraction digits. Estimates below a * hundredth of a cent still read as a number rather than collapsing to $0.00, @@ -141,7 +160,12 @@ export function formatUsageReport(data: UsageReportInput): string[] { lines.push(""); lines.push(...table( ["PROVIDER", "REQUESTS", "TOKENS", "EST. COST"], - providers.map(row => [row.provider, count(row.requests), count(row.totalTokens), usd(row.estimatedCostUsd)]), + providers.map(row => [ + row.provider, + count(row.requests), + countWithCache(row.totalTokens, row.cacheReadInputTokens ?? row.cachedInputTokens), + usd(row.estimatedCostUsd), + ]), )); } @@ -163,7 +187,7 @@ export function formatUsageReport(data: UsageReportInput): string[] { // wrong conclusion. Mark it rather than presenting it as a single identity. row.ambiguous ? `${terminalText(row.accountLogLabel)} (ambiguous)` : terminalText(row.accountLogLabel), count(row.requests), - count(row.totalTokens), + countWithCache(row.totalTokens, row.cacheReadInputTokens ?? row.cachedInputTokens), usd(row.estimatedCostUsd), ]), )); @@ -175,7 +199,13 @@ export function formatUsageReport(data: UsageReportInput): string[] { const shown = models.slice(0, MAX_MODEL_ROWS); lines.push(...table( ["MODEL", "PROVIDER", "REQUESTS", "TOKENS", "EST. COST"], - shown.map(row => [row.model ?? "-", row.provider, count(row.requests), count(row.totalTokens), usd(row.estimatedCostUsd)]), + shown.map(row => [ + row.model ?? "-", + row.provider, + count(row.requests), + countWithCache(row.totalTokens, row.cacheReadInputTokens ?? row.cachedInputTokens), + usd(row.estimatedCostUsd), + ]), )); if (models.length > shown.length) { lines.push(`... ${models.length - shown.length} more (use --json)`); diff --git a/src/lib/redact.ts b/src/lib/redact.ts index f9e3e3ec6a..2206a9baa9 100644 --- a/src/lib/redact.ts +++ b/src/lib/redact.ts @@ -252,6 +252,13 @@ const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [ [/((?:"(?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$3`], // Raw JSON "token" field values (Copilot token exchange bodies echo the credential here). [/(("token"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$4`], + // Cognition/Devin session keys, and the bare JWTs several providers hand out. + // A Connect EOS trailer can quote the request that carried the key, and the + // rules above only fire on a label — `Bearer`, `api_key=`, `"token":` — which + // a quoted proto field does not have. `eyJ` is the base64url of `{"`, so the + // JWT rule needs a real three-segment shape and does not match ordinary prose. + [/\bdevin-session-token\$[A-Za-z0-9._~+/=-]{8,}/g, REDACTED_SECRET], + [/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]*/g, REDACTED_SECRET], [/\b(arn:aws:[A-Za-z0-9_-]+:[A-Za-z0-9-]*:\d{12}:[A-Za-z0-9_/:+=,.@-]+)\b/g, REDACTED_SECRET], ]; diff --git a/src/oauth/devin-cli.ts b/src/oauth/devin-cli.ts index abb3cd14ab..78107d409d 100644 --- a/src/oauth/devin-cli.ts +++ b/src/oauth/devin-cli.ts @@ -70,10 +70,14 @@ export function devinCliCredentialsPath( if (override && (override.startsWith("/") || /^[A-Za-z]:[\\/]/.test(override))) return override; const paths = platform === "win32" ? win32 : posix; if (platform === "win32") { - const appData = env.APPDATA ?? paths.join(homedir(), "AppData", "Roaming"); + // `??` treats an empty APPDATA as set, and join("", "devin", …) is a path + // relative to whatever directory the proxy was started in — so a file planted + // there would import as the operator's own CLI session. An empty or + // whitespace-only value is an absent value. + const appData = env.APPDATA?.trim() || paths.join(homedir(), "AppData", "Roaming"); return paths.join(appData, "devin", "credentials.toml"); } - const dataHome = env.XDG_DATA_HOME ?? paths.join(homedir(), ".local", "share"); + const dataHome = env.XDG_DATA_HOME?.trim() || paths.join(homedir(), ".local", "share"); return paths.join(dataHome, "devin", "credentials.toml"); } @@ -83,30 +87,60 @@ export interface DevinCliCredentialFile { } /** - * Read the two keys that matter, and nothing else. + * Upper bound on the credential file we are willing to parse. * - * The measured file is four flat `key = "value"` lines: no tables, no comments, - * no single quotes. A line matcher is therefore enough and a TOML dependency is - * not, and the quoted form is required rather than optional — an unquoted - * matcher would pass its own fixtures and miss the real file. + * The measured file is four short lines. Reading an arbitrarily large file into + * a string and running two global-ish regexes over it is work we never need to + * do, and a file this size is not the CLI's. + */ +const DEVIN_CLI_CREDENTIALS_MAX_BYTES = 64 * 1024; + +/** + * Why the import has no credential, for the one error message the caller owns. * - * Returns undefined rather than throwing so the caller owns the one error - * message. Nothing here ever puts the file's contents into a thrown value. + * `missing` and `unreadable` used to collapse into the same `undefined`, so a + * permission error on an existing file was reported as "not signed in" and sent + * the operator to `devin auth login`, which does not fix it. */ -export function readDevinCliCredentialFile(deps: DevinCliLoginDeps = {}): DevinCliCredentialFile | undefined { +export type DevinCliCredentialOutcome = + | { kind: "ok"; file: DevinCliCredentialFile } + | { kind: "missing" } + | { kind: "unreadable" } + | { kind: "incomplete" }; + +export function readDevinCliCredentialOutcome(deps: DevinCliLoginDeps = {}): DevinCliCredentialOutcome { const path = devinCliCredentialsPath(deps.env, deps.platform); const exists = deps.exists ?? existsSync; - if (!exists(path)) return undefined; + if (!exists(path)) return { kind: "missing" }; let raw: string; try { raw = (deps.read ?? ((p: string) => readFileSync(p, "utf8")))(path); } catch { - return undefined; + // Nothing from the error is repeated: it carries the path, and an EACCES + // message is not worth the risk of echoing anything read off disk. + return { kind: "unreadable" }; } + if (raw.length > DEVIN_CLI_CREDENTIALS_MAX_BYTES) return { kind: "unreadable" }; const apiKey = raw.match(/^\s*windsurf_api_key\s*=\s*"([^"]+)"/m)?.[1]?.trim(); const apiServerUrl = raw.match(/^\s*api_server_url\s*=\s*"([^"]+)"/m)?.[1]?.trim(); - if (!apiKey || !apiServerUrl) return undefined; - return { apiKey, apiServerUrl }; + if (!apiKey || !apiServerUrl) return { kind: "incomplete" }; + return { kind: "ok", file: { apiKey, apiServerUrl } }; +} + +/** + * Read the two keys that matter, and nothing else. + * + * The measured file is four flat `key = "value"` lines: no tables, no comments, + * no single quotes. A line matcher is therefore enough and a TOML dependency is + * not, and the quoted form is required rather than optional — an unquoted + * matcher would pass its own fixtures and miss the real file. + * + * Returns undefined rather than throwing so the caller owns the one error + * message. Nothing here ever puts the file's contents into a thrown value. + */ +export function readDevinCliCredentialFile(deps: DevinCliLoginDeps = {}): DevinCliCredentialFile | undefined { + const outcome = readDevinCliCredentialOutcome(deps); + return outcome.kind === "ok" ? outcome.file : undefined; } /** True when a signed-in CLI credential is readable. Used for status, never for auth. */ @@ -119,16 +153,29 @@ export async function loginDevinCli( _opts?: DevinCliLoginOpts, deps: DevinCliLoginDeps = {}, ): Promise { - const file = readDevinCliCredentialFile(deps); - if (!file) { - // Deliberately names no path contents and no parsed value. A Connect error - // can echo a request, and redactSecretString does not recognise a bare JWT - // or a devin-session-token, which is why register-user.ts refuses to repeat - // error bodies; the same caution applies to anything thrown from here. + const outcome = readDevinCliCredentialOutcome(deps); + // Each branch deliberately names no path contents and no parsed value. A + // Connect error can echo a request, and redactSecretString does not recognise + // a bare JWT or a devin-session-token, which is why register-user.ts refuses + // to repeat error bodies; the same caution applies to anything thrown here. + if (outcome.kind === "unreadable") { + // The file is there and we could not read it, so `devin auth login` is the + // wrong instruction: it would succeed and change nothing. + throw new Error( + "Found a Devin CLI credential file but could not read it. Check its permissions and size, then try again.", + ); + } + if (outcome.kind === "incomplete") { + throw new Error( + "The Devin CLI credential file is missing its session key or API server URL. Run `devin auth login` again to rewrite it.", + ); + } + if (outcome.kind === "missing") { throw new Error( `No signed-in Devin CLI session found. ${DEVIN_CLI_INSTALL_HINT} Then run \`devin auth login\` and try again.`, ); } + const file = outcome.file; // The host comes off disk and then receives the key, so it passes the same // allowlist as the RegisterUser host. An unallowlisted value falls back to the // default rather than becoming an exfiltration target. diff --git a/src/server/management/oauth-account-routes.ts b/src/server/management/oauth-account-routes.ts index 89f80e5f88..480e8c7842 100644 --- a/src/server/management/oauth-account-routes.ts +++ b/src/server/management/oauth-account-routes.ts @@ -78,6 +78,24 @@ import type { MetricUnavailableReason, TokPerSecondResult, CostEstimateReason, C import type { ManagementContext } from "./context"; import { readManagementJsonBody, readManagementJsonBodyOr, rethrowManagementBodyTooLarge } from "./body"; import { codexAccountNamespaceProviderCollisionError } from "../../codex/account-namespace-match"; + +/** + * Provider ids that share the Devin cloud-direct client, and therefore share its + * process-memory caches. + * + * `devin` signs in through RegisterUser and `devin-cli` imports a signed-in local + * CLI session, but both hand the same api_key to the same client, so one cache + * serves both and one of them clearing it is not enough. + */ +function isDevinCloudDirectProvider(provider: string): boolean { + return provider === "devin" || provider === "devin-cli"; +} + +async function clearDevinCloudDirectCaches(): Promise { + const { clearCachedUserJwt, clearCachedCatalog } = await import("../../adapters/devin/cloud-direct"); + clearCachedUserJwt(); + clearCachedCatalog(); +} import { ACCOUNT_IMPORT_DEADLINE_MS, ACCOUNT_IMPORT_MAX_REQUEST_BYTES } from "../../oauth/account-import"; import { readBoundedJsonRequestBody } from "../request-decompress"; @@ -255,14 +273,12 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const { clearProviderQuotaCache, clearAccountQuotaCache } = await import("../../providers/quota"); clearProviderQuotaCache(); clearAccountQuotaCache(provider); - if (provider === "devin") { - // The cached user_jwt's payload contains the api_key, and the catalog is - // keyed by that key. Without this they outlive the credential in process - // memory until the JWT's own ~24 minute expiry. - const { clearCachedUserJwt, clearCachedCatalog } = await import("../../adapters/devin/cloud-direct"); - clearCachedUserJwt(); - clearCachedCatalog(); - } + // The cached user_jwt's payload contains the api_key, and the catalog is + // keyed by that key. Without this they outlive the credential in process + // memory until the JWT's own ~24 minute expiry. `devin` and `devin-cli` + // share one cache, so gating on `devin` alone left a CLI-imported key's JWT + // resident after its own logout. + if (isDevinCloudDirectProvider(provider)) await clearDevinCloudDirectCaches(); return jsonResponse({ success: true }); } @@ -685,6 +701,10 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const { clearProviderQuotaCache, clearAccountQuotaCache } = await import("../../providers/quota"); clearProviderQuotaCache(); clearAccountQuotaCache(provider); + // Same reasoning as logout. Removing the last account for a provider used to + // leave the JWT and catalog in memory, because only the logout route cleared + // them. + if (isDevinCloudDirectProvider(provider)) await clearDevinCloudDirectCaches(); return jsonResponse({ ok: true }); } diff --git a/tests/cli/cli-usage-report.test.ts b/tests/cli/cli-usage-report.test.ts index 5399137a57..1be03f68f2 100644 --- a/tests/cli/cli-usage-report.test.ts +++ b/tests/cli/cli-usage-report.test.ts @@ -404,3 +404,28 @@ describe("ocx logs --follow output contract", () => { } }); }); + +describe("cached tokens in the per-row tables", () => { + test("a provider and model row name their cache-read subset", () => { + // The summary line has always said "cached N". The tables below it printed a + // bare total, so a mostly-cached provider looked like an ordinary one. + const lines = formatUsageReport({ + range: "today", + summary: { requests: 1, totalTokens: 58_000, cachedInputTokens: 57_000 }, + providers: [{ provider: "devin-cli", requests: 1, totalTokens: 58_000, cacheReadInputTokens: 57_000 }], + models: [{ provider: "devin-cli", model: "swe-2", requests: 1, totalTokens: 58_000, cachedInputTokens: 57_000 }], + }).join("\n"); + expect(lines).toContain("58,000 (cached 57,000)"); + expect(lines.match(/cached 57,000/g)?.length).toBeGreaterThanOrEqual(2); + }); + + test("a provider that reports no cache keeps a bare total", () => { + const lines = formatUsageReport({ + range: "today", + summary: { requests: 1, totalTokens: 58_000 }, + providers: [{ provider: "xai", requests: 1, totalTokens: 58_000 }], + }).join("\n"); + expect(lines).toContain("58,000"); + expect(lines).not.toContain("cached"); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 9ea5f32928..5e57040565 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -549,6 +549,7 @@ "grok-sync.test.ts": "providers/xai", "grok-writer-boundary.test.ts": "providers/xai", "gui-api-error.test.ts": "gui", + "gui-format-tokens-cache.test.ts": "gui", "gui-management-session.test.ts": "gui", "gui-pair-capability.test.ts": "gui", "gui-pair-client.test.ts": "gui", diff --git a/tests/gui/gui-format-tokens-cache.test.ts b/tests/gui/gui-format-tokens-cache.test.ts new file mode 100644 index 0000000000..fe38fb81da --- /dev/null +++ b/tests/gui/gui-format-tokens-cache.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test"; +import { formatTokens, formatTokensWithCache } from "../../gui/src/format-tokens"; + +/** + * A cached request's total is mostly cache. The logs table has always shown the + * total with a stacked `c `; every other surface printed the total alone, + * which reads as a different, smaller request than the rows beside it. + */ +describe("formatTokensWithCache", () => { + test("renders the cached subset beside the total in both number scales", () => { + expect(formatTokensWithCache(58_000, 57_000, "ko")).toBe("5.8만 c5.7만"); + expect(formatTokensWithCache(58_000, 57_000, "en")).toBe("58K c57K"); + expect(formatTokensWithCache(58_000, 57_000, "zh")).toBe("5.8万 c5.7万"); + }); + + test("a provider that reports no cache is left exactly as it was", () => { + for (const cached of [undefined, 0, Number.NaN]) { + expect(formatTokensWithCache(58_000, cached, "ko")).toBe(formatTokens(58_000, "ko")); + } + // A negative count is nonsense rather than a cache miss; treat it as absent. + expect(formatTokensWithCache(58_000, -1, "en")).toBe("58K"); + }); + + test("a turn served entirely from cache still shows the marker", () => { + // This is the most cached row on the page. Hiding the companion when the + // subset equals the total would blank exactly the case worth showing. + expect(formatTokensWithCache(57_000, 57_000, "en")).toBe("57K c57K"); + }); +}); diff --git a/tests/lib/redact.test.ts b/tests/lib/redact.test.ts index 26a4912a27..f15e4aa30d 100644 --- a/tests/lib/redact.test.ts +++ b/tests/lib/redact.test.ts @@ -550,3 +550,26 @@ test("redact-folding folds colon confusables with aligned offsets and stays a ze const source = readFileSync(repoPath("src/lib/redact-folding.ts"), "utf8"); expect(source).not.toMatch(/^import /m); }); + +describe("bare credential shapes with no label to key off", () => { + test("a Devin session token is masked wherever it appears", () => { + // A Connect EOS trailer can quote the request that carried the key, and the + // labelled rules never fire on a quoted proto field. + const token = "devin-session-token$eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ.c2ln"; + const masked = redactSecretString(`permission_denied: api_key ${token} was rejected`); + expect(masked).not.toContain("devin-session-token$eyJ"); + expect(masked).not.toContain("eyJhbGciOiJIUzI1NiJ9"); + }); + + test("a bare JWT is masked, and ordinary prose is not", () => { + const masked = redactSecretString("token eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJ1c2VyIn0.c2lnbmF0dXJl here"); + expect(masked).not.toContain("eyJhbGciOiJIUzI1NiJ9"); + for (const benign of [ + "version 1.2.3 shipped", + "see src/lib/redact.ts for the rules", + "a.b.c", + ]) { + expect(redactSecretString(benign)).toBe(benign); + } + }); +}); diff --git a/tests/providers/devin-cli-login.test.ts b/tests/providers/devin-cli-login.test.ts index ba82c033f1..766e28eca2 100644 --- a/tests/providers/devin-cli-login.test.ts +++ b/tests/providers/devin-cli-login.test.ts @@ -5,6 +5,7 @@ import { devinCliSignedIn, loginDevinCli, readDevinCliCredentialFile, + readDevinCliCredentialOutcome, refreshDevinCliToken, } from "../../src/oauth/devin-cli"; import { resolveDevinApiServer } from "../../src/oauth/devin"; @@ -135,3 +136,74 @@ describe("devin tenant selection is provider-scoped", () => { }); }); + +describe("devin-cli credential path and read bounds", () => { + const okFile = [ + 'windsurf_api_key = "devin-session-token$eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ.sig"', + 'api_server_url = "https://server.codeium.com"', + "", + ].join("\n"); + + test("an empty XDG_DATA_HOME or APPDATA does not become a cwd-relative path", () => { + // `??` treated "" as set, so join("", "devin", …) resolved against whatever + // directory the proxy was started in, and a planted file there would import + // as the operator's own CLI session. + // The fallback reads the real home directory rather than env.HOME, so the + // assertion is on shape: absolute, and under the home data dir. + for (const empty of ["", " "]) { + const resolved = devinCliCredentialsPath({ HOME: "/home/u", XDG_DATA_HOME: empty }, "linux"); + expect(resolved.startsWith("/")).toBe(true); + expect(resolved.endsWith("/.local/share/devin/credentials.toml")).toBe(true); + } + const win = devinCliCredentialsPath({ APPDATA: "" }, "win32"); + expect(win.endsWith("AppData\\Roaming\\devin\\credentials.toml")).toBe(true); + expect(win.startsWith("devin")).toBe(false); + }); + + test("a present-but-unreadable file is not reported as a missing sign-in", async () => { + const deps = { + env: { HOME: "/home/u", XDG_DATA_HOME: "/home/u/.local/share" }, + platform: "linux" as NodeJS.Platform, + exists: () => true, + read: () => { throw new Error("EACCES: permission denied"); }, + }; + expect(readDevinCliCredentialOutcome(deps)).toEqual({ kind: "unreadable" }); + // "run devin auth login" would succeed and change nothing, so the two + // outcomes must not share one message. + await expect(loginDevinCli({} as OAuthController, undefined, deps)).rejects.toThrow(/could not read it/); + await expect(loginDevinCli({} as OAuthController, undefined, { ...deps, exists: () => false })) + .rejects.toThrow(/No signed-in Devin CLI session/); + }); + + test("a file past the parse bound is refused rather than scanned", () => { + const deps = { + env: { HOME: "/home/u", XDG_DATA_HOME: "/home/u/.local/share" }, + platform: "linux" as NodeJS.Platform, + exists: () => true, + read: () => okFile + "#".repeat(64 * 1024), + }; + expect(readDevinCliCredentialOutcome(deps).kind).toBe("unreadable"); + expect(readDevinCliCredentialFile(deps)).toBeUndefined(); + }); + + test("a file with only one of the two keys names the incomplete case", () => { + const deps = { + env: { HOME: "/home/u", XDG_DATA_HOME: "/home/u/.local/share" }, + platform: "linux" as NodeJS.Platform, + exists: () => true, + read: () => 'api_server_url = "https://server.codeium.com"\n', + }; + expect(readDevinCliCredentialOutcome(deps).kind).toBe("incomplete"); + }); + + test("no thrown message repeats the key", async () => { + const deps = { + env: { HOME: "/home/u", XDG_DATA_HOME: "/home/u/.local/share" }, + platform: "linux" as NodeJS.Platform, + exists: () => true, + read: () => { throw new Error("EACCES"); }, + }; + const err = await loginDevinCli({} as OAuthController, undefined, deps).catch((e: unknown) => e); + expect(String(err)).not.toContain("devin-session-token"); + }); +}); diff --git a/tests/providers/devin-hardening.test.ts b/tests/providers/devin-hardening.test.ts index 07a5302ce7..6d85a61007 100644 --- a/tests/providers/devin-hardening.test.ts +++ b/tests/providers/devin-hardening.test.ts @@ -5,7 +5,19 @@ import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseU import { registerUser } from "../../src/oauth/devin/register-user"; import { anySignal } from "../../src/lib/abort"; import { buildGetChatMessageRequestForTests } from "../../src/adapters/devin/cloud-direct/chat"; +import { decodeModelUsageStats } from "../../src/adapters/devin/cloud-direct/chat"; +import { CloudChatError, decodeChatFrame } from "../../src/adapters/devin/cloud-direct/chat"; +import { connectTrailerHttpStatus } from "../../src/adapters/devin/cloud-direct/chat"; +import { devinErrorClassification, mergeDevinUsage } from "../../src/adapters/devin"; import { iterFields } from "../../src/adapters/devin/cloud-direct/wire"; +import { buildMetadata, normalizeDevinSessionToken } from "../../src/adapters/devin/cloud-direct/metadata"; + +/** Tag -> field for one encoded proto message. */ +function iterFieldMap(buf: Buffer): Record { + const out: Record = {}; + for (const f of iterFields(buf)) out[f.num] = { wire: f.wire, value: f.value }; + return out; +} const FAKE_TOKEN = "eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ1c2VyLTEifQ.c2lnbmF0dXJl"; @@ -247,3 +259,188 @@ describe("devin cloud request shape", () => { expect((metadata[31]?.value as Buffer).length).toBe(732); }); }); + +describe("devin session-token normalization", () => { + test("a bare JWT regains the prefix the service reads", () => { + const jwt = "eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ.sig"; + expect(normalizeDevinSessionToken(jwt)).toBe("devin-session-token$" + jwt); + // Without this, the key goes out verbatim and Cognition answers with an + // opaque permission_denied, which reads as a revoked account. + const metadata = iterFieldMap(buildMetadata({ + apiKey: jwt, requestId: 1, sessionId: "s", triggerId: "t", cloudChatShape: true, + })); + expect((metadata[3]?.value as Buffer).toString("utf8")).toBe("devin-session-token$" + jwt); + }); + + test("every other key format this field has carried passes through untouched", () => { + for (const key of [ + "devin-session-token$eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ.sig", + "3f2504e0-4f89-11d3-9a0c-0305e82c3301", + "sk-ws-01-abcdef", + "cog_abcdef", + "", + ]) { + expect(normalizeDevinSessionToken(key)).toBe(key); + } + }); +}); + +describe("devin ModelUsageStats decode (response field 7)", () => { + function varint(num: number, value: number): Buffer { + const out: number[] = [(num << 3) | 0]; + let v = value; + do { const b = v & 0x7f; v = Math.floor(v / 128); out.push(v > 0 ? b | 0x80 : b); } while (v > 0); + return Buffer.from(out); + } + const stats = (input: number, output: number, write: number, read: number) => + Buffer.concat([varint(2, input), varint(3, output), varint(4, write), varint(5, read)]); + + test("an exclusive frame folds cache into the inclusive input this repo reports", () => { + // 1k fresh + 57k cache read is the 58k prompt the user sees as one number. + const u = decodeModelUsageStats(stats(1_000, 200, 0, 57_000)); + expect(u?.promptTokens).toBe(58_000); + expect(u?.cachedInputTokens).toBe(57_000); + expect(u?.totalTokens).toBe(58_200); + }); + + test("an already-inclusive frame is left alone rather than inflated", () => { + const u = decodeModelUsageStats(stats(58_000, 200, 0, 57_000)); + expect(u?.promptTokens).toBe(58_000); + expect(u?.cachedInputTokens).toBe(57_000); + // normalizeCostTokens only rejects read + write > input, so an inflated + // input would pass validation and bill cache at the uncached rate. + expect(u!.cachedInputTokens! + (u!.cacheCreationInputTokens ?? 0)).toBeLessThanOrEqual(u!.promptTokens!); + }); + + test("cache write counts as prompt too, and an empty message decodes to nothing", () => { + const u = decodeModelUsageStats(stats(1_000, 0, 4_000, 0)); + expect(u?.promptTokens).toBe(5_000); + expect(u?.cacheCreationInputTokens).toBe(4_000); + expect(decodeModelUsageStats(Buffer.alloc(0))).toBeNull(); + }); +}); + +describe("devin frame-level usage precedence and classification", () => { + // Tags above 15 need a multi-byte varint: field 28 wire 2 is 226, and + // writing that as one raw byte sets the continuation bit and swallows the + // next byte. + function uvarint(value: number): number[] { + const out: number[] = []; + let v = value; + do { const b = v & 0x7f; v = Math.floor(v / 128); out.push(v > 0 ? b | 0x80 : b); } while (v > 0); + return out; + } + function varint(num: number, value: number): Buffer { + return Buffer.from([...uvarint((num << 3) | 0), ...uvarint(value)]); + } + function lenDelim(num: number, payload: Buffer): Buffer { + return Buffer.concat([Buffer.from([...uvarint((num << 3) | 2), ...uvarint(payload.length)]), payload]); + } + // ResponseDimensionGroup carrying a cumulative metric whose uid reads like a + // metric id — the shape the old decoder mined for usage. + function displayGroup(uid: string, value: number): Buffer { + const f32 = Buffer.alloc(5); + f32.writeUInt8((2 << 3) | 5, 0); + f32.writeFloatLE(value, 1); + const entry = Buffer.concat([lenDelim(4, f32), lenDelim(5, Buffer.from(uid, "utf8"))]); + return lenDelim(2, entry); + } + + test("field 7 suppresses the display rows and is reported before finish", () => { + const stats = Buffer.concat([varint(2, 1_000), varint(3, 200), varint(4, 0), varint(5, 57_000)]); + const frame = Buffer.concat([ + lenDelim(7, stats), + varint(5, 2), // stop_reason STOP_PATTERN + lenDelim(28, displayGroup("input_tokens", 999)), // the wrong, display-derived number + ]); + const events = [...decodeChatFrame(frame)]; + const usages = events.filter(e => e.kind === "usage"); + expect(usages).toHaveLength(1); + expect(usages[0]!.promptTokens).toBe(58_000); + expect(usages[0]!.cachedInputTokens).toBe(57_000); + // Ahead of finish, so ordering does not depend on where the service puts + // the field. + expect(events.findIndex(e => e.kind === "usage")) + .toBeLessThan(events.findIndex(e => e.kind === "finish")); + }); + + test("a frame with no field 7 still falls back to the display rows", () => { + const frame = lenDelim(28, Buffer.concat([ + displayGroup("input_tokens", 4_000), + displayGroup("output_tokens", 100), + ])); + const usages = [...decodeChatFrame(frame)].filter(e => e.kind === "usage"); + expect(usages).toHaveLength(1); + expect(usages[0]!.promptTokens).toBe(4_000); + }); +}); + +describe("devin usage merging and error classification", () => { + test("a later partial frame cannot zero an earlier count, and the total stays derived", () => { + const merged = mergeDevinUsage( + { inputTokens: 58_000, outputTokens: 200, totalTokens: 58_200, cachedInputTokens: 57_000 }, + { inputTokens: 58_000, outputTokens: 900 }, + ); + expect(merged.cachedInputTokens).toBe(57_000); + expect(merged.outputTokens).toBe(900); + // Taking the max of two totals alongside per-field maxima would leave + // 58,200 here, which no longer equals input + output. + expect(merged.totalTokens).toBe(58_900); + }); + + test("an HTTP status on the cloud error becomes a structured classification", () => { + expect(devinErrorClassification(new CloudChatError("x", undefined, undefined, 429))) + .toEqual({ status: 429, errorType: "rate_limit_error", retryable: true }); + expect(devinErrorClassification(new CloudChatError("x", undefined, undefined, 401))) + .toEqual({ status: 401, errorType: "authentication_error", retryable: false }); + expect(devinErrorClassification(new CloudChatError("x", undefined, undefined, 503))) + .toEqual({ status: 503, retryable: true }); + // A Connect trailer carries no status, so it keeps the older inference path. + expect(devinErrorClassification(new CloudChatError("x", "resource_exhausted"))).toEqual({}); + }); +}); + +describe("connect trailer to HTTP status", () => { + test("a cap delivered as permission_denied is a 429, not a 403", () => { + // Cognition sends the account cap through the same code as an ACL denial. + // Classified 403 the client retries straight into a live cap. + expect(connectTrailerHttpStatus("permission_denied", "Your limit will reset in 13 minutes")).toBe(429); + expect(connectTrailerHttpStatus("permission_denied", "Reached overall message rate limit")).toBe(429); + // An ordinary denial stays a denial. + expect(connectTrailerHttpStatus("permission_denied", "an internal error occurred")).toBe(403); + }); + + test("the remaining Connect codes map to the status core acts on", () => { + expect(connectTrailerHttpStatus("unauthenticated", "")).toBe(401); + expect(connectTrailerHttpStatus("resource_exhausted", "")).toBe(429); + expect(connectTrailerHttpStatus("unavailable", "")).toBe(503); + expect(connectTrailerHttpStatus("deadline_exceeded", "")).toBe(504); + expect(connectTrailerHttpStatus("invalid_argument", "")).toBe(400); + expect(connectTrailerHttpStatus("internal", "")).toBe(502); + // An unknown code keeps the older message-inference path rather than + // asserting a status nobody measured. + expect(connectTrailerHttpStatus("some_new_code", "")).toBeUndefined(); + expect(connectTrailerHttpStatus(undefined, "")).toBeUndefined(); + }); + + test("a trailer status reaches the adapter's structured classification", () => { + const err = new CloudChatError("capped", "permission_denied", "abc", connectTrailerHttpStatus("permission_denied", "Your limit will reset in 3 minutes")); + expect(devinErrorClassification(err)).toEqual({ status: 429, errorType: "rate_limit_error", retryable: true }); + }); +}); + +describe("devin status classification across the newly reachable trailer codes", () => { + const cls = (status: number) => devinErrorClassification(new CloudChatError("x", undefined, undefined, status)); + + test("a request the service will not accept is never retried", () => { + expect(cls(400)).toEqual({ status: 400, retryable: false }); + expect(cls(404)).toEqual({ status: 404, retryable: false }); + // 501 is the one 5xx a second attempt cannot change. + expect(cls(501)).toEqual({ status: 501, retryable: false }); + }); + + test("a timeout or an unavailable service is retryable", () => { + expect(cls(503)).toEqual({ status: 503, retryable: true }); + expect(cls(504)).toEqual({ status: 504, retryable: true }); + }); +});