From 0b5a842249d578f7829a5f5e839e07be4e03a67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 07:25:35 +0200 Subject: [PATCH 01/11] feat(cli-sessions-v2): expose total_cost_microdollars on getWithRuntimeState Add the persisted per-session cost column to the getWithRuntimeState output schema and return projection, thread it through FetchedSessionData as an optional field, and populate it in the mobile fetchSession mapping. The mobile session detail screen will use it to render the same canonical cost the list reads from the Postgres column. Web fetchSession behavior is unchanged (the new field is optional). --- apps/mobile/src/components/agents/mobile-session-manager.ts | 1 + apps/web/src/lib/cloud-agent-sdk/session-manager.ts | 1 + apps/web/src/routers/cli-sessions-v2-router.ts | 2 ++ 3 files changed, 4 insertions(+) diff --git a/apps/mobile/src/components/agents/mobile-session-manager.ts b/apps/mobile/src/components/agents/mobile-session-manager.ts index 4b2b380e2b..69594cd270 100644 --- a/apps/mobile/src/components/agents/mobile-session-manager.ts +++ b/apps/mobile/src/components/agents/mobile-session-manager.ts @@ -271,6 +271,7 @@ export function createMobileAgentSessionManager({ initialMessageId: rs?.initialMessageId ?? null, associatedPr: sessionResult.associatedPr, runtimeAgents: rs?.runtimeAgents, + totalCostMicrodollars: sessionResult.total_cost_microdollars, }; }, }); diff --git a/apps/web/src/lib/cloud-agent-sdk/session-manager.ts b/apps/web/src/lib/cloud-agent-sdk/session-manager.ts index e23aa6e6c7..078ddc549f 100644 --- a/apps/web/src/lib/cloud-agent-sdk/session-manager.ts +++ b/apps/web/src/lib/cloud-agent-sdk/session-manager.ts @@ -152,6 +152,7 @@ type FetchedSessionData = { /** Custom modes exposed by this session's profile stack (slug + name, plus optional model and thinking-effort overrides). */ runtimeAgents?: Array<{ slug: string; name: string; model?: string; variant?: string }>; associatedPr: AssociatedPrData | null; + totalCostMicrodollars?: number | null; }; type PrepareInput = { diff --git a/apps/web/src/routers/cli-sessions-v2-router.ts b/apps/web/src/routers/cli-sessions-v2-router.ts index 2a109c643f..ec3c32d13f 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.ts @@ -828,6 +828,7 @@ export const cliSessionsV2Router = createTRPCRouter({ created_at: z.coerce.date(), updated_at: z.coerce.date(), version: z.number(), + total_cost_microdollars: z.number().nullable(), // Runtime state from DO (null for CLI sessions without cloud_agent_session_id) runtimeState: baseGetSessionNextOutputSchema.nullable(), // Associated GitHub pull request for this session's branch, if any. @@ -941,6 +942,7 @@ export const cliSessionsV2Router = createTRPCRouter({ created_at: session.created_at, updated_at: session.updated_at, version: session.version, + total_cost_microdollars: session.total_cost_microdollars, runtimeState, associatedPr: formatAssociatedPr(row), }; From 646c7fc74c28ea96502bbc7d4ae4ec976199a897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 07:25:38 +0200 Subject: [PATCH 02/11] fix(mobile): show one canonical session cost on list and detail The list rendered the persisted column at 2 decimals with a { describe('getHeaderSummary', () => { it('returns null when there is no completed assistant context usage', () => { - expect(getHeaderSummary(undefined, 0.08)).toBeNull(); + expect(getHeaderSummary(undefined, 80_000)).toBeNull(); expect(getHeaderSummary(undefined, 0)).toBeNull(); + expect(getHeaderSummary(undefined, null)).toBeNull(); }); it('shows percentage as primary and cost as secondary when capacity is known', () => { - const summary = getHeaderSummary(info({ percentage: 42 }), 0.08); + const summary = getHeaderSummary(info({ percentage: 42 }), 80_000); expect(summary).toEqual({ primary: '42%', - secondary: '$0.0800', + secondary: '$0.08', hasCost: true, tone: 'primary', }); }); - it('omits the secondary cost when cost is zero', () => { - const summary = getHeaderSummary(info({ percentage: 10 }), 0); - expect(summary).toEqual({ primary: '10%', hasCost: false, tone: 'primary' }); + it('omits the secondary cost when cost is zero or null', () => { + expect(getHeaderSummary(info({ percentage: 10 }), 0)).toEqual({ + primary: '10%', + hasCost: false, + tone: 'primary', + }); + expect(getHeaderSummary(info({ percentage: 10 }), null)).toEqual({ + primary: '10%', + hasCost: false, + tone: 'primary', + }); }); it('uses percentage as primary and a warning tone at 75-89%', () => { - const summary = getHeaderSummary(info({ percentage: 80 }), 0.5); + const summary = getHeaderSummary(info({ percentage: 80 }), 500_000); expect(summary?.primary).toBe('80%'); expect(summary?.tone).toBe('warning'); - expect(summary?.secondary).toBe('$0.5000'); + expect(summary?.secondary).toBe('$0.50'); }); it('keeps the real overflow percentage visible (does not clamp above 100) and uses a destructive tone', () => { const summary = getHeaderSummary( info({ contextTokens: 250_000, contextWindow: 200_000, percentage: 125 }), - 1 + 1_000_000 ); expect(summary?.primary).toBe('125%'); expect(summary?.tone).toBe('destructive'); @@ -187,11 +196,11 @@ describe('getHeaderSummary', () => { it('falls back to compact tokens and a neutral tone when capacity is unknown', () => { const summary = getHeaderSummary( info({ contextWindow: undefined, percentage: undefined, contextTokens: 32_418 }), - 0.12 + 120_000 ); expect(summary).toEqual({ primary: '32.4K', - secondary: '$0.1200', + secondary: '$0.12', hasCost: true, tone: 'neutral', }); @@ -210,7 +219,7 @@ describe('getContextSheetContent', () => { it('describes exact usage and remaining tokens when capacity is known', () => { const content = getContextSheetContent( info({ contextTokens: 84_000, contextWindow: 200_000, percentage: 42 }), - 0.08 + 80_000 ); expect(content.usedTokens).toBe('84,000'); expect(content.windowTokens).toBe('200,000'); @@ -218,7 +227,7 @@ describe('getContextSheetContent', () => { expect(content.percentage).toBe('42%'); expect(content.remainingTokens).toBe('116,000'); expect(content.remainingPercentage).toBe('58%'); - expect(content.cost).toBe('$0.0800'); + expect(content.cost).toBe('$0.08'); expect(content.tone).toBe('primary'); }); @@ -230,7 +239,7 @@ describe('getContextSheetContent', () => { expect(content.percentage).toBe('125%'); expect(content.remainingTokens).toBe('0'); expect(content.remainingPercentage).toBe('0%'); - expect(content.cost).toBe('$0.0000'); + expect(content.cost).toBeNull(); expect(content.tone).toBe('destructive'); }); @@ -244,27 +253,28 @@ describe('getContextSheetContent', () => { expect(content.windowUnavailable).toBe(true); expect(content.percentage).toBeNull(); expect(content.remainingTokens).toBeNull(); - expect(content.cost).toBe('$0.0000'); + expect(content.cost).toBeNull(); expect(content.windowUnavailableLabel).toBe('Context-window size unavailable'); expect(content.tone).toBe('neutral'); }); - it('shows the cost line as $0.0000 when total cost is zero', () => { + it('returns null cost when total cost is zero (sheet omits the Total cost row)', () => { const content = getContextSheetContent(info({ percentage: 20 }), 0); - expect(content.cost).toBe('$0.0000'); + expect(content.cost).toBeNull(); }); }); describe('getMetricsAccessibilityLabel', () => { - it('includes exact usage, real percentage, and tap intent when capacity is known with cost', () => { + it('includes exact usage, real percentage, humanized cost, and tap intent when capacity is known with cost', () => { const label = getMetricsAccessibilityLabel( info({ contextTokens: 84_000, contextWindow: 200_000, percentage: 42 }), - 0.08 + 80_000 ); expect(label).toContain('84,000'); expect(label).toContain('200,000'); expect(label).toContain('42%'); - expect(label).toContain('$0.0800'); + expect(label).toContain('8 cents'); + expect(label).not.toContain('$'); expect(label.toLowerCase()).toContain('context details'); }); @@ -274,6 +284,7 @@ describe('getMetricsAccessibilityLabel', () => { 0 ); expect(label).not.toContain('$'); + expect(label).not.toContain('cost'); }); it('switches to the unavailable-capacity copy and omits percentage/cost when not available', () => { @@ -287,12 +298,13 @@ describe('getMetricsAccessibilityLabel', () => { expect(label).not.toContain('$'); }); - it('includes the positive cost in the unknown-capacity case', () => { + it('includes the humanized positive cost in the unknown-capacity case', () => { const label = getMetricsAccessibilityLabel( info({ contextWindow: undefined, percentage: undefined, contextTokens: 32_418 }), - 0.12 + 120_000 ); - expect(label).toContain('$0.1200'); + expect(label).toContain('12 cents'); + expect(label).not.toContain('$'); }); it('preserves the real overflow percentage in the known-capacity case (125%)', () => { @@ -306,11 +318,11 @@ describe('getMetricsAccessibilityLabel', () => { }); describe('pure integration fallback', () => { - it('shows the cost-only header when there is no completed assistant context usage', () => { + it('returns null summary when there is no completed assistant context usage', () => { // Mirrors the SessionDetailContent integration: when resolveSessionContextInfo - // returns undefined the header should keep the legacy positive cost text - // (no context control, no sheet) rather than an empty header. - const summary = getHeaderSummary(undefined, 0.08); + // returns undefined the header falls through to SessionContextCostFallback + // rather than a context control. + const summary = getHeaderSummary(undefined, 80_000); expect(summary).toBeNull(); }); }); diff --git a/apps/mobile/src/components/agents/context-usage-display.ts b/apps/mobile/src/components/agents/context-usage-display.ts index 2d37304276..dcaf1c866d 100644 --- a/apps/mobile/src/components/agents/context-usage-display.ts +++ b/apps/mobile/src/components/agents/context-usage-display.ts @@ -1,5 +1,8 @@ import { type SessionContextInfo } from '@/lib/session-context-info'; +import { formatSessionTotalCost } from './session-list-helpers'; +import { formatSpokenCost } from './session-row-accessibility-label'; + export type ContextTone = 'primary' | 'warning' | 'destructive' | 'neutral'; const NUMBER_FORMAT = new Intl.NumberFormat('en-US'); @@ -91,7 +94,7 @@ type HeaderSummary = { export function getHeaderSummary( info: SessionContextInfo | undefined, - totalCost: number + totalCostMicrodollars: number | null ): HeaderSummary | null { if (!info) { return null; @@ -99,10 +102,11 @@ export function getHeaderSummary( const tone = getContextTone(info.percentage); const primary = info.percentage !== undefined ? `${info.percentage}%` : formatCompactTokens(info.contextTokens); - if (totalCost <= 0) { + const secondary = formatSessionTotalCost(totalCostMicrodollars); + if (secondary === null) { return { primary, hasCost: false, tone }; } - return { primary, secondary: formatCost(totalCost), hasCost: true, tone }; + return { primary, secondary, hasCost: true, tone }; } type ContextSheetContent = { @@ -114,17 +118,17 @@ type ContextSheetContent = { percentage: string | null; remainingTokens: string | null; remainingPercentage: string | null; - cost: string; + cost: string | null; tone: ContextTone; }; export function getContextSheetContent( info: SessionContextInfo, - totalCost: number + totalCostMicrodollars: number | null ): ContextSheetContent { const tone = getContextTone(info.percentage); const usedTokens = formatExactTokens(info.contextTokens); - const cost = formatCost(totalCost); + const cost = formatSessionTotalCost(totalCostMicrodollars); if (info.contextWindow === undefined) { return { usedTokens, @@ -159,8 +163,12 @@ export function getContextSheetContent( }; } -export function getMetricsAccessibilityLabel(info: SessionContextInfo, totalCost: number): string { - const costPart = totalCost > 0 ? `, cost ${formatCost(totalCost)}` : ''; +export function getMetricsAccessibilityLabel( + info: SessionContextInfo, + totalCostMicrodollars: number | null +): string { + const spoken = formatSpokenCost(totalCostMicrodollars); + const costPart = spoken ? `, cost ${spoken}` : ''; if (info.contextWindow === undefined) { return `Context ${formatExactTokens(info.contextTokens)} tokens, window unavailable${costPart}. Tap to view context details.`; } diff --git a/apps/mobile/src/components/agents/session-context-metrics.tsx b/apps/mobile/src/components/agents/session-context-metrics.tsx index 4387c2b289..3849a1d9bb 100644 --- a/apps/mobile/src/components/agents/session-context-metrics.tsx +++ b/apps/mobile/src/components/agents/session-context-metrics.tsx @@ -12,10 +12,12 @@ import { getHeaderSummary, getMetricsAccessibilityLabel, } from './context-usage-display'; +import { formatSessionTotalCost } from './session-list-helpers'; +import { formatSpokenCost } from './session-row-accessibility-label'; type SessionContextMetricsProps = { info: SessionContextInfo; - totalCost: number; + totalCostMicrodollars: number | null; onPress: () => void; }; @@ -35,16 +37,16 @@ function toneTextClass(tone: ContextTone): string { export function SessionContextMetrics({ info, - totalCost, + totalCostMicrodollars, onPress, }: Readonly) { - const summary = getHeaderSummary(info, totalCost); + const summary = getHeaderSummary(info, totalCostMicrodollars); if (!summary) { return null; } const tone = getContextTone(info.percentage); const arcFraction = getArcFraction(info.percentage); - const accessibilityLabel = getMetricsAccessibilityLabel(info, totalCost); + const accessibilityLabel = getMetricsAccessibilityLabel(info, totalCostMicrodollars); return ( ) { - if (totalCost <= 0) { +// Preserves the positive-cost header text when no completed context usage +// exists. Marked noninteractive; VoiceOver reads the humanized cost. +export function SessionContextCostFallback({ + totalCostMicrodollars, +}: Readonly<{ totalCostMicrodollars: number | null }>) { + const visible = formatSessionTotalCost(totalCostMicrodollars); + if (visible === null) { return null; } + const spoken = formatSpokenCost(totalCostMicrodollars); return ( - ${totalCost.toFixed(4)} + {visible} ); } diff --git a/apps/mobile/src/components/agents/session-context-sheet.tsx b/apps/mobile/src/components/agents/session-context-sheet.tsx index fb42571a50..e8bc9eb7aa 100644 --- a/apps/mobile/src/components/agents/session-context-sheet.tsx +++ b/apps/mobile/src/components/agents/session-context-sheet.tsx @@ -34,7 +34,8 @@ type SessionContextSheetProps = { info: SessionContextInfo; modelDisplay: string; providerDisplay: string; - totalCost: number; + totalCostMicrodollars: number | null; + breakdownCostUsd: number; messages: StoredMessage[]; modelOptions: SessionModelOption[]; onClose: () => void; @@ -59,18 +60,19 @@ export function SessionContextSheet({ info, modelDisplay, providerDisplay, - totalCost, + totalCostMicrodollars, + breakdownCostUsd, messages, modelOptions, onClose, }: Readonly) { const insets = useSafeAreaInsets(); - const content = getContextSheetContent(info, totalCost); + const content = getContextSheetContent(info, totalCostMicrodollars); const tone = getContextTone(info.percentage); const arcFraction = getArcFraction(info.percentage); const breakdown = useMemo( - () => getSessionCostBreakdown(messages, totalCost), - [messages, totalCost] + () => getSessionCostBreakdown(messages, breakdownCostUsd), + [messages, breakdownCostUsd] ); // Render-only filter: totals/subagent residual still use the full breakdown. const visibleModels = useMemo( @@ -147,11 +149,13 @@ export function SessionContextSheet({ {providerDisplay} - - - {content.cost} - - + {content.cost !== null ? ( + + + {content.cost} + + + ) : null} Usage reflects the latest completed assistant response. diff --git a/apps/mobile/src/components/agents/session-cost-formatters.test.ts b/apps/mobile/src/components/agents/session-cost-formatters.test.ts index 73027236b4..76ac21f87c 100644 --- a/apps/mobile/src/components/agents/session-cost-formatters.test.ts +++ b/apps/mobile/src/components/agents/session-cost-formatters.test.ts @@ -1,62 +1,55 @@ import { describe, expect, it } from 'vitest'; import { formatSpokenCost } from './session-row-accessibility-label'; -import { composeStoredSessionSpokenMeta, formatSessionListCost } from './session-list-helpers'; +import { + composeStoredSessionSpokenMeta, + formatSessionTotalCost, + selectSessionCostInputs, +} from './session-list-helpers'; /** - * F1 — list cost (visible + spoken formatters). + * Canonical session cost formatters (visible + spoken + selector). * - * Microdollars is the count of $0.000001 units (USD × 1,000,000). Both - * helpers collapse null/0/non-finite inputs to `null` so the row can omit - * the cost segment entirely. Visible and spoken forms are kept independent - * because the visible row wants compact "$0.12" while VoiceOver wants - * words ("12 cents"). + * Microdollars is the count of $0.000001 units (USD × 1,000,000). Visible and + * spoken agree on the omit band; the selector takes max(persisted, live). */ -describe('formatSessionListCost (visible)', () => { +describe('formatSessionTotalCost (visible)', () => { it('returns null for null', () => { - expect(formatSessionListCost(null)).toBeNull(); + expect(formatSessionTotalCost(null)).toBeNull(); }); it('returns null for undefined', () => { - expect(formatSessionListCost(undefined)).toBeNull(); + expect(formatSessionTotalCost(undefined)).toBeNull(); }); - it('returns null for zero', () => { - expect(formatSessionListCost(0)).toBeNull(); - }); - - it('returns null for negative values', () => { - expect(formatSessionListCost(-1)).toBeNull(); - }); - - it('returns null for non-finite numbers', () => { - expect(formatSessionListCost(Number.NaN)).toBeNull(); - expect(formatSessionListCost(Number.POSITIVE_INFINITY)).toBeNull(); - expect(formatSessionListCost(Number.NEGATIVE_INFINITY)).toBeNull(); + it('returns null for zero, negative, and non-finite', () => { + expect(formatSessionTotalCost(0)).toBeNull(); + expect(formatSessionTotalCost(-1)).toBeNull(); + expect(formatSessionTotalCost(Number.NaN)).toBeNull(); + expect(formatSessionTotalCost(Number.POSITIVE_INFINITY)).toBeNull(); }); - it('renders sub-half-cent values as "<$0.01" (not "$0.00")', () => { - // microdollars < 5000 → usd < 0.005 - expect(formatSessionListCost(1)).toBe('<$0.01'); - expect(formatSessionListCost(4999)).toBe('<$0.01'); + it('omits 1..49 µ$ (would render a false $0.0000)', () => { + expect(formatSessionTotalCost(1)).toBeNull(); + expect(formatSessionTotalCost(25)).toBeNull(); + expect(formatSessionTotalCost(49)).toBeNull(); }); - it('rounds at the half-cent boundary (5000 micro → "$0.01", not "<$0.01")', () => { - // usd = 0.005 → toFixed(2) = "0.01" - expect(formatSessionListCost(5000)).toBe('$0.01'); + it('formats sub-half-cent values to four decimals', () => { + expect(formatSessionTotalCost(50)).toBe('$0.0001'); + expect(formatSessionTotalCost(3081)).toBe('$0.0031'); + expect(formatSessionTotalCost(4999)).toBe('$0.0050'); }); - it('formats a typical sub-dollar value to two decimal places', () => { - expect(formatSessionListCost(120_000)).toBe('$0.12'); + it('switches to two decimals at the half-cent threshold (inclusive)', () => { + expect(formatSessionTotalCost(5000)).toBe('$0.01'); + expect(formatSessionTotalCost(9999)).toBe('$0.01'); + expect(formatSessionTotalCost(10_000)).toBe('$0.01'); + expect(formatSessionTotalCost(13_113)).toBe('$0.01'); }); - it('formats whole-dollar values', () => { - expect(formatSessionListCost(1_000_000)).toBe('$1.00'); - expect(formatSessionListCost(12_500_000)).toBe('$12.50'); - }); - - it('formats a multi-dollar value with cents', () => { - expect(formatSessionListCost(3_420_000)).toBe('$3.42'); + it('formats multi-dollar values to two decimals', () => { + expect(formatSessionTotalCost(1_234_567)).toBe('$1.23'); }); }); @@ -83,13 +76,22 @@ describe('formatSpokenCost (a11y)', () => { expect(formatSpokenCost(Number.NEGATIVE_INFINITY)).toBeNull(); }); - it('returns null when the value rounds to zero cents', () => { - expect(formatSpokenCost(4000)).toBeNull(); - expect(formatSpokenCost(4999)).toBeNull(); + it('returns null in the visible omit band (1..49 µ$)', () => { + expect(formatSpokenCost(1)).toBeNull(); + expect(formatSpokenCost(25)).toBeNull(); + expect(formatSpokenCost(49)).toBeNull(); + }); + + it('speaks sub-half-cent values as fractional cents', () => { + expect(formatSpokenCost(50)).toBe('0.01 cents'); + expect(formatSpokenCost(3081)).toBe('0.31 cents'); + expect(formatSpokenCost(4000)).toBe('0.4 cents'); + expect(formatSpokenCost(4999)).toBe('0.5 cents'); }); it('rounds at the half-cent boundary (5000 micro → "1 cent")', () => { expect(formatSpokenCost(5000)).toBe('1 cent'); + expect(formatSpokenCost(13_113)).toBe('1 cent'); }); it('speaks a single sub-dollar cent in singular form', () => { @@ -123,10 +125,77 @@ describe('formatSpokenCost (a11y)', () => { }); }); +describe('selectSessionCostInputs', () => { + it('returns null total when both inputs are absent', () => { + expect(selectSessionCostInputs(null, 0)).toEqual({ + totalMicrodollars: null, + breakdownCostUsd: 0, + }); + expect(selectSessionCostInputs(undefined, 0)).toEqual({ + totalMicrodollars: null, + breakdownCostUsd: 0, + }); + }); + + it('uses persisted when live is zero', () => { + expect(selectSessionCostInputs(120_000, 0)).toEqual({ + totalMicrodollars: 120_000, + breakdownCostUsd: 0, + }); + }); + + it('uses live µ$ when persisted is null', () => { + expect(selectSessionCostInputs(null, 0.12)).toEqual({ + totalMicrodollars: 120_000, + breakdownCostUsd: 0.12, + }); + }); + + it('picks the larger of persisted and live', () => { + expect(selectSessionCostInputs(500_000, 0.1)).toEqual({ + totalMicrodollars: 500_000, + breakdownCostUsd: 0.1, + }); + expect(selectSessionCostInputs(100_000, 0.5)).toEqual({ + totalMicrodollars: 500_000, + breakdownCostUsd: 0.5, + }); + }); + + it('returns the shared value when equal', () => { + expect(selectSessionCostInputs(120_000, 0.12)).toEqual({ + totalMicrodollars: 120_000, + breakdownCostUsd: 0.12, + }); + }); + + it('treats non-finite and negative inputs as zero', () => { + expect(selectSessionCostInputs(Number.NaN, Number.NaN)).toEqual({ + totalMicrodollars: null, + breakdownCostUsd: 0, + }); + expect(selectSessionCostInputs(-100, -0.5)).toEqual({ + totalMicrodollars: null, + breakdownCostUsd: 0, + }); + expect(selectSessionCostInputs(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY)).toEqual({ + totalMicrodollars: null, + breakdownCostUsd: 0, + }); + }); + + it('never leaks the combined total into breakdownCostUsd (composition gate)', () => { + // persisted $0.50 wins over live $0.001, but breakdown stays the live sum + const result = selectSessionCostInputs(500_000, 0.001); + expect(result.totalMicrodollars).toBe(500_000); + expect(result.breakdownCostUsd).toBe(0.001); + }); +}); + /** * End-to-end spoken meta composition — the exact wiring the row uses. * These tests would fail if the row composed spoken meta with the visible - * formatter (`formatSessionListCost` → "$0.12") instead of the humanized + * formatter (`formatSessionTotalCost` → "$0.12") instead of the humanized * spoken formatter (`formatSpokenCost` → "12 cents"). */ describe('composeStoredSessionSpokenMeta (spoken wiring)', () => { @@ -135,9 +204,9 @@ describe('composeStoredSessionSpokenMeta (spoken wiring)', () => { expect(result).toBe('cost 12 cents, 5 minutes ago'); }); - it('omits the cost phrase for a sub-half-cent charge (time-only)', () => { + it('composes a fractional-cent cost with the spoken time', () => { const result = composeStoredSessionSpokenMeta(formatSpokenCost(4000), '2 hours ago'); - expect(result).toBe('2 hours ago'); + expect(result).toBe('cost 0.4 cents, 2 hours ago'); }); it('omits the cost phrase when cost is null (time-only)', () => { diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 40a4b8a01f..7702042331 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -29,6 +29,7 @@ import { SessionContextMetrics, } from '@/components/agents/session-context-metrics'; import { SessionContextSheet } from '@/components/agents/session-context-sheet'; +import { selectSessionCostInputs } from '@/components/agents/session-list-helpers'; import { buildRemoteAttachmentParts } from '@/components/agents/mobile-session-manager-helpers'; import { buildRemoteAttachmentPartsWithRetryableFeedback, @@ -129,6 +130,10 @@ export function SessionDetailContent({ const activeQuestion = useAtomValue(manager.atoms.activeQuestion); const activePermission = useAtomValue(manager.atoms.activePermission); const totalCost = useAtomValue(manager.atoms.totalCost); + const { totalMicrodollars, breakdownCostUsd } = selectSessionCostInputs( + fetchedData?.kiloSessionId === sessionId ? fetchedData.totalCostMicrodollars : null, + totalCost + ); const getChildMessages = useAtomValue(manager.atoms.childMessages); const getChildSessionHydrationState = useAtomValue(manager.atoms.childSessionHydrationState); const pendingMessages = useAtomValue(manager.atoms.pendingMessages); @@ -218,7 +223,7 @@ export function SessionDetailContent({ const headerRight = contextInfo ? ( { setOpenContextSheetIdentity({ sessionId, @@ -228,7 +233,7 @@ export function SessionDetailContent({ }} /> ) : ( - + ); const sheetMountState = getContextSheetMountState( contextInfo, @@ -653,7 +658,8 @@ export function SessionDetailContent({ info={sheetMountState.info} modelDisplay={contextModelAndProvider.model} providerDisplay={contextModelAndProvider.provider} - totalCost={totalCost} + totalCostMicrodollars={totalMicrodollars} + breakdownCostUsd={breakdownCostUsd} messages={messages} modelOptions={modelOptions} onClose={() => { diff --git a/apps/mobile/src/components/agents/session-list-helpers.ts b/apps/mobile/src/components/agents/session-list-helpers.ts index f3e21a0be9..1103f3e3db 100644 --- a/apps/mobile/src/components/agents/session-list-helpers.ts +++ b/apps/mobile/src/components/agents/session-list-helpers.ts @@ -67,33 +67,53 @@ export function formatMeta(timestamp: string): string { } /** - * Visible cost segment for a stored session row. + * Canonical visible cost for every mobile session surface (list row, detail + * header, context sheet total). * - * Returns `null` whenever the row should not display a cost — caller omits - * the segment entirely. Inputs that are `null`, `undefined`, zero, or not - * finite (defensive against unexpected shapes from the server) all collapse - * to `null` so the list row shows the timestamp alone. + * Returns `null` whenever the surface should not display a cost — caller + * omits the segment/row entirely. Inputs that are `null`, `undefined`, zero, + * non-positive, or not finite all collapse to `null`. * - * Otherwise the microdollar count is converted to USD and formatted as a - * two-decimal dollar amount (e.g. `$0.12`). Sub-half-cent values render as - * `"<$0.01"` so the smallest visible charge is unambiguous — a $0.001 row - * is not silently rendered as `$0.00`. + * Conversion: microdollars → USD. At or above half a cent (`>= 5000` µ$), + * two-decimal dollars (e.g. `$0.01`, `$1.23`). Below half a cent, four + * decimals (e.g. `$0.0031`); values that would render `$0.0000` (1..49 µ$) + * are omitted instead of a false zero. */ -export function formatSessionListCost(microdollars: number | null | undefined): string | null { - if (microdollars === null || microdollars === undefined) { - return null; - } - if (!Number.isFinite(microdollars)) { - return null; - } - if (microdollars <= 0) { +export function formatSessionTotalCost(microdollars: number | null | undefined): string | null { + if (microdollars == null || !Number.isFinite(microdollars) || microdollars <= 0) { return null; } const usd = microdollars / 1_000_000; - if (usd < 0.005) { - return '<$0.01'; + if (usd >= 0.005) { + return `$${usd.toFixed(2)}`; } - return `$${usd.toFixed(2)}`; + const fine = `$${usd.toFixed(4)}`; + return fine === '$0.0000' ? null : fine; +} + +/** + * Derive the canonical session total (max of persisted DB µ$ and live client + * USD sum) plus the sanitized live breakdown input. + * + * Session cost is monotonically non-decreasing, so both inputs are lower + * bounds and `max` is strictly closer to truth. `breakdownCostUsd` is always + * the sanitized live sum — never the combined total — so per-model breakdown + * math stays aligned with the live message stream. + */ +export function selectSessionCostInputs( + persistedMicrodollars: number | null | undefined, + liveUsd: number +): { totalMicrodollars: number | null; breakdownCostUsd: number } { + const persisted = + typeof persistedMicrodollars === 'number' && Number.isFinite(persistedMicrodollars) + ? Math.max(0, persistedMicrodollars) + : 0; + const live = Number.isFinite(liveUsd) ? Math.max(0, Math.round(liveUsd * 1_000_000)) : 0; + const total = Math.max(persisted, live); + return { + totalMicrodollars: total > 0 ? total : null, + breakdownCostUsd: Number.isFinite(liveUsd) ? Math.max(0, liveUsd) : 0, + }; } /** diff --git a/apps/mobile/src/components/agents/session-row-accessibility-label.ts b/apps/mobile/src/components/agents/session-row-accessibility-label.ts index a513ef2509..43242cfaa6 100644 --- a/apps/mobile/src/components/agents/session-row-accessibility-label.ts +++ b/apps/mobile/src/components/agents/session-row-accessibility-label.ts @@ -43,24 +43,22 @@ export function formatSpokenTimeAgo(timestamp: string): string { /** * Speech-friendly cost formatter. * - * Mirrors the visible cost segment on the stored session list row, but in a - * form VoiceOver reads as words rather than the literal `"$0.12"`. Inputs - * that are `null`, `undefined`, zero, or not finite collapse to `null` so - * the caller can omit the cost phrase from the spoken meta entirely - * (matching the visible row, which shows the timestamp alone when there - * is no cost). + * Mirrors the visible cost segment (`formatSessionTotalCost`) but in a form + * VoiceOver reads as words rather than the literal `"$0.12"`. Inputs that + * are `null`, `undefined`, zero, or not finite collapse to `null` so the + * caller can omit the cost phrase from the spoken meta entirely (matching + * the visible surface, which omits cost when there is none). * - * Otherwise the microdollar count is converted to USD, rounded to whole - * cents, and spoken as: - * - under $1 → `" cent(s)"` - * - $1+ → `" dollar(s)"` plus `" cent(s)"` only when the - * cents component is non-zero - * - * A value that rounds to zero cents (e.g. a $0.004 sub-half-cent charge) - * returns `null` so the spoken form omits an amount that rounds to zero - * whole cents — whole-cent granularity for speech. This intentionally - * diverges from the visible formatter, which shows `"<$0.01"` for a - * sub-half-cent charge. + * Spoken and visible agree on the omit band (1..49 µ$ → null). Branching + * uses the same half-cent threshold as the visible formatter (`>= 5000` µ$): + * - `>= 5000` µ$: whole-cent rounding, then + * under $1 → `" cent(s)"` + * $1+ → `" dollar(s)"` plus `" cent(s)"` only when the + * cents component is non-zero + * - below 5000 µ$: fractional cents to 2 decimal places with trailing + * zeros trimmed (e.g. `0.4 cents` for 4000 µ$), always plural `cents`. + * Below half a cent the spoken form is the exact humanized reading of + * the visible 4-decimal string. */ export function formatSpokenCost(microdollars: number | null | undefined): string | null { if (microdollars === null || microdollars === undefined) { @@ -72,20 +70,30 @@ export function formatSpokenCost(microdollars: number | null | undefined): strin if (microdollars <= 0) { return null; } - const cents = Math.round(microdollars / 10_000); - if (cents <= 0) { - return null; - } - if (cents < 100) { - return `${cents} ${cents === 1 ? 'cent' : 'cents'}`; + if (microdollars >= 5000) { + const cents = Math.round(microdollars / 10_000); + if (cents <= 0) { + return null; + } + if (cents < 100) { + return `${cents} ${cents === 1 ? 'cent' : 'cents'}`; + } + const dollars = Math.floor(cents / 100); + const remainder = cents % 100; + const dollarPart = `${dollars} ${dollars === 1 ? 'dollar' : 'dollars'}`; + if (remainder === 0) { + return dollarPart; + } + return `${dollarPart} ${remainder} ${remainder === 1 ? 'cent' : 'cents'}`; } - const dollars = Math.floor(cents / 100); - const remainder = cents % 100; - const dollarPart = `${dollars} ${dollars === 1 ? 'dollar' : 'dollars'}`; - if (remainder === 0) { - return dollarPart; + // Sub-half-cent: fractional cents, 2 dp, trim trailing zeros. + const fractional = microdollars / 10_000; + const rounded = Math.round(fractional * 100) / 100; + if (rounded <= 0) { + return null; } - return `${dollarPart} ${remainder} ${remainder === 1 ? 'cent' : 'cents'}`; + const trimmed = String(rounded); + return `${trimmed} cents`; } type SessionRowAccessibilityLabelInputs = { diff --git a/apps/mobile/src/components/agents/session-row.tsx b/apps/mobile/src/components/agents/session-row.tsx index e66fbcd285..f307d0b22e 100644 --- a/apps/mobile/src/components/agents/session-row.tsx +++ b/apps/mobile/src/components/agents/session-row.tsx @@ -16,7 +16,7 @@ import { composeStoredSessionSpokenMeta, composeStoredSessionVisibleMeta, formatMeta, - formatSessionListCost, + formatSessionTotalCost, storedSessionEyebrowLabel, } from './session-list-helpers'; import { @@ -159,7 +159,7 @@ export function StoredSessionRow({ // When a cost is present, both forms fold it in first (matches the row's // "$0.12 · time" order). Needs-input sessions have no persisted cost. const visibleMeta = composeStoredSessionVisibleMeta( - formatSessionListCost(session.total_cost_microdollars), + formatSessionTotalCost(session.total_cost_microdollars), formatMeta(timestamp) ); const spokenMeta = needsInput From 202a27e32546471497f62e3f27b428cf04788495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 07:25:40 +0200 Subject: [PATCH 03/11] fix(session-ingest): persist session cost before the O11Y metrics RPC The best-effort total_cost_microdollars write sat after the unguarded O11Y ingestSessionMetrics call, so an analytics binding failure skipped the cost persist entirely and the column stayed NULL. Reorder so the cost write runs first; the O11Y call stays unguarded and the metricsEmitted dedup is unchanged, preserving the existing alarm retry semantics. --- .../session-ingest/src/dos/SessionIngestDO.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/services/session-ingest/src/dos/SessionIngestDO.ts b/services/session-ingest/src/dos/SessionIngestDO.ts index b7f1f515fc..cd351e6f02 100644 --- a/services/session-ingest/src/dos/SessionIngestDO.ts +++ b/services/session-ingest/src/dos/SessionIngestDO.ts @@ -704,17 +704,9 @@ export class SessionIngestDO extends DurableObject { } } - await this.env.O11Y.ingestSessionMetrics({ - kiloUserId, - sessionId, - ingestVersion, - model, - ...metrics, - }); - // Best-effort persist the per-session total cost to Postgres so the session - // list can surface it. Runs once per close under the metricsEmitted dedup. - // Failures are logged and swallowed — must never break metrics emission. + // list can surface it. Runs before the O11Y RPC so an analytics failure cannot + // skip it. Failures are logged and swallowed — must never break metrics emission. try { if (Number.isFinite(metrics.totalCost)) { const totalCostMicrodollars = Math.max(0, Math.round(metrics.totalCost * 1_000_000)); @@ -737,6 +729,14 @@ export class SessionIngestDO extends DurableObject { }); } + await this.env.O11Y.ingestSessionMetrics({ + kiloUserId, + sessionId, + ingestVersion, + model, + ...metrics, + }); + // Mark metrics as emitted to prevent duplicates this.db .insert(ingestMeta) From d0b1f7fca82d54da7c390d59c8462afcbfe4b3c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 12:54:43 +0200 Subject: [PATCH 04/11] chore: retrigger code review From c0abf6046d56913714c7d7c0db6031d431521a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 14:11:37 +0200 Subject: [PATCH 05/11] feat(worker-utils): add hasOrganizationAccess membership check Standalone worker-side query mirroring the session-access predicates in cloud-agent-session-access.ts: a direct organization_memberships row for (kilo_user_id, organization_id) and a non-soft-deleted organization. Parent-organization inheritance and kilocode_users.is_admin are deliberately excluded, matching every existing worker-side check. --- packages/worker-utils/src/index.ts | 2 + .../src/organization-membership.test.ts | 59 +++++++++++++++++++ .../src/organization-membership.ts | 46 +++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 packages/worker-utils/src/organization-membership.test.ts create mode 100644 packages/worker-utils/src/organization-membership.ts diff --git a/packages/worker-utils/src/index.ts b/packages/worker-utils/src/index.ts index c719fb17ea..4e79e1a60b 100644 --- a/packages/worker-utils/src/index.ts +++ b/packages/worker-utils/src/index.ts @@ -196,3 +196,5 @@ export type { SecurityFindingAuditSnapshotSource, SecurityFindingAuditWriterDb, } from './security-finding-audit.js'; + +export { hasOrganizationAccess } from './organization-membership.js'; diff --git a/packages/worker-utils/src/organization-membership.test.ts b/packages/worker-utils/src/organization-membership.test.ts new file mode 100644 index 0000000000..700674111d --- /dev/null +++ b/packages/worker-utils/src/organization-membership.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { hasOrganizationAccess } from './organization-membership.js'; + +type MembershipFixture = { kind: 'member' } | { kind: 'non-member' } | { kind: 'soft-deleted' }; + +function createMembershipDb(fixture: MembershipFixture) { + const rows = + fixture.kind === 'member' + ? [{ id: 'mem_1' }] + : // non-member and soft-deleted org both yield no row: the join requires + // membership + organizations.deleted_at IS NULL. + []; + + const limit = vi.fn(async () => rows); + const where = vi.fn(() => ({ limit })); + const innerJoin = vi.fn(() => ({ where })); + const from = vi.fn(() => ({ innerJoin })); + const select = vi.fn(() => ({ from })); + + return { select, from, innerJoin, where, limit, rows }; +} + +describe('hasOrganizationAccess', () => { + it('returns true when the user has a direct membership in a live org', async () => { + const db = createMembershipDb({ kind: 'member' }); + + await expect( + hasOrganizationAccess(db as never, { kiloUserId: 'usr_1', organizationId: 'org_1' }) + ).resolves.toBe(true); + + expect(db.select).toHaveBeenCalledOnce(); + expect(db.from).toHaveBeenCalledOnce(); + expect(db.innerJoin).toHaveBeenCalledOnce(); + expect(db.where).toHaveBeenCalledOnce(); + expect(db.limit).toHaveBeenCalledWith(1); + }); + + it('returns false when the user has no membership row', async () => { + const db = createMembershipDb({ kind: 'non-member' }); + + await expect( + hasOrganizationAccess(db as never, { kiloUserId: 'usr_1', organizationId: 'org_1' }) + ).resolves.toBe(false); + }); + + it('returns false when the user is a member of a soft-deleted org', async () => { + // Soft-deleted orgs are filtered by isNull(organizations.deleted_at) on the + // join, so the query returns no row — same as non-membership to the caller. + const db = createMembershipDb({ kind: 'soft-deleted' }); + + await expect( + hasOrganizationAccess(db as never, { + kiloUserId: 'usr_1', + organizationId: 'org_deleted', + }) + ).resolves.toBe(false); + }); +}); diff --git a/packages/worker-utils/src/organization-membership.ts b/packages/worker-utils/src/organization-membership.ts new file mode 100644 index 0000000000..4828ec3329 --- /dev/null +++ b/packages/worker-utils/src/organization-membership.ts @@ -0,0 +1,46 @@ +import type { WorkerDb } from '@kilocode/db/client'; +import { organization_memberships, organizations } from '@kilocode/db/schema'; +import { and, eq, isNull } from 'drizzle-orm'; + +type OrganizationMembershipDb = Pick; + +/** + * True when the user has a direct membership row for the organization and the + * organization is not soft-deleted. Mirrors worker session-access predicates in + * `cloud-agent-session-access.ts` as a standalone query (no session join). + * + * Deliberately excluded: + * - Parent-organization inherited roles — honoured only by the tRPC path + * (`apps/web/src/routers/organizations/utils.ts`), restricted to + * owner/billing_manager. No worker-side check considers them; adding + * inheritance here would make this the single most permissive worker check in + * the repo, in a security fix. The pre-existing gap is uniform across every + * worker path and is not this PR's to change. + * - `kilocode_users.is_admin` — no worker session-access path consults it, and + * session-ingest JWT auth deliberately discards every JWT claim except + * `kiloUserId`. + */ +export async function hasOrganizationAccess( + db: OrganizationMembershipDb, + params: { kiloUserId: string; organizationId: string } +): Promise { + const rows = await db + .select({ id: organization_memberships.id }) + .from(organization_memberships) + .innerJoin( + organizations, + and( + eq(organizations.id, organization_memberships.organization_id), + isNull(organizations.deleted_at) + ) + ) + .where( + and( + eq(organization_memberships.organization_id, params.organizationId), + eq(organization_memberships.kilo_user_id, params.kiloUserId) + ) + ) + .limit(1); + + return rows[0] !== undefined; +} From 964eaabbd2ff7184c01c70d9f521f771f75fef40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 14:11:56 +0200 Subject: [PATCH 06/11] fix(session-ingest): refuse unauthorized organization_id on metadata ingest applyMetadataChanges persisted cli_sessions_v2.organization_id verbatim from the client-supplied kilo_meta.orgId with no membership check, so any caller could re-tenant their own session into any existing organization, and a nonexistent organization aborted the whole metadata batch on the FK while the client still got HTTP 200. Gate the write on hasOrganizationAccess inside the same transaction; on refusal drop only organization_id, persist the rest of the batch, and warn. Access-cache invalidation and changedNonStatus now key on an actually-applied org write so a refusal is not reported as a scope change. --- .../src/ingest/metadata.test.ts | 325 ++++++++++++++++++ .../session-ingest/src/ingest/metadata.ts | 53 ++- .../session-ingest/src/queue-consumer.test.ts | 9 +- 3 files changed, 377 insertions(+), 10 deletions(-) diff --git a/services/session-ingest/src/ingest/metadata.test.ts b/services/session-ingest/src/ingest/metadata.test.ts index e8cfc88602..0dfbb15b07 100644 --- a/services/session-ingest/src/ingest/metadata.test.ts +++ b/services/session-ingest/src/ingest/metadata.test.ts @@ -31,8 +31,10 @@ vi.mock('../session-events', () => ({ })); import { getWorkerDb } from '@kilocode/db/client'; +import { getSessionAccessCacheDO } from '../dos/SessionAccessCacheDO'; import { notifyUserSessionEvent } from '../session-events'; import { + applyMetadataChanges, CLI_DISCONNECT_ATTENTION_RESET_STATUS, resetAttentionStatusOnCliDisconnect, } from './metadata'; @@ -105,6 +107,116 @@ function createTransactionDb(options: { return { transaction, select, applyUpdate, updateSet, updateWhere }; } +type ApplyMetadataDbOptions = { + /** Membership join row count (0 = unauthorized / missing / soft-deleted). */ + membershipRows?: number; + /** When set, the next non-lock session select is treated as a parent lookup. */ + parentExists?: boolean; + initialStatus?: string | null; + rowMissing?: boolean; +}; + +/** + * Fluent drizzle double for applyMetadataChanges. + * + * Distinguishes query kinds by chain shape: + * - membership (hasOrganizationAccess): select → from → innerJoin → where → limit + * - status lock: select → from → where → limit → for('update') + * - parent / read-back: select → from → where → limit (awaited without for) + */ +function createApplyMetadataDb(options: ApplyMetadataDbOptions = {}) { + const updateSets: unknown[] = []; + const updateWhere = vi.fn(async () => undefined); + const updateSet = vi.fn((values: unknown) => { + updateSets.push(values); + return { where: updateWhere }; + }); + // Named without the substring "update" so oxlint drizzle rules do not flag test spies. + const applyUpdate = vi.fn(() => ({ set: updateSet })); + + const queryLog: Array<'session-lock' | 'membership' | 'parent' | 'read-back'> = []; + let parentLookupDone = false; + + function persistedSessionRow() { + return { + session_id: 'ses_1', + created_at: '2026-01-01T00:00:00.000Z', + updated_at: '2026-01-01T00:00:01.000Z', + title: 'T', + created_on_platform: 'cli', + organization_id: null, + git_url: null, + git_branch: null, + parent_session_id: null, + status: options.initialStatus ?? 'idle', + status_updated_at: '2026-07-25T00:00:00.000Z', + }; + } + + function sessionLimitResult() { + // Dual-mode: `.for('update')` ⇒ status lock; bare await ⇒ parent lookup or read-back. + let settled: Promise | undefined; + + const resolveWithoutFor = () => { + if (options.parentExists !== undefined && !parentLookupDone) { + parentLookupDone = true; + queryLog.push('parent'); + return options.parentExists ? [{ session_id: 'ses_parent' }] : []; + } + queryLog.push('read-back'); + return options.rowMissing ? [] : [persistedSessionRow()]; + }; + + const thenable = { + for: vi.fn(() => { + queryLog.push('session-lock'); + const rows = options.rowMissing + ? [] + : ([{ status: options.initialStatus ?? 'idle' }] satisfies StatusRow[]); + settled = Promise.resolve(rows); + return settled; + }), + then(onFulfilled: (value: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) { + settled ??= Promise.resolve(resolveWithoutFor()); + return settled.then(onFulfilled, onRejected); + }, + }; + return thenable; + } + + const select = vi.fn(() => ({ + from: vi.fn(() => ({ + innerJoin: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: vi.fn(async () => { + queryLog.push('membership'); + const count = options.membershipRows ?? 0; + return count > 0 ? [{ id: 'mem_1' }] : []; + }), + })), + })), + where: vi.fn(() => ({ + limit: vi.fn(() => sessionLimitResult()), + })), + })), + })); + + const transaction = vi.fn(async (fn: (tx: unknown) => Promise) => + fn({ select, update: applyUpdate }) + ); + + return { + transaction, + select, + applyUpdate, + updateSet, + updateWhere, + updateSets, + queryLog, + membershipQueryCount: () => queryLog.filter(k => k === 'membership').length, + }; +} + describe('resetAttentionStatusOnCliDisconnect', () => { beforeEach(() => { vi.mocked(getWorkerDb).mockReset(); @@ -208,3 +320,216 @@ describe('resetAttentionStatusOnCliDisconnect', () => { expect(notifyUserSessionEvent).not.toHaveBeenCalled(); }); }); + +describe('applyMetadataChanges', () => { + const env = { HYPERDRIVE: { connectionString: 'postgres://unused' } } as never; + const cacheRemove = vi.fn(async () => undefined); + + beforeEach(() => { + vi.mocked(getWorkerDb).mockReset(); + vi.mocked(notifyUserSessionEvent).mockReset(); + vi.mocked(getSessionAccessCacheDO).mockReset(); + cacheRemove.mockReset(); + vi.mocked(getSessionAccessCacheDO).mockReturnValue({ + remove: cacheRemove, + } as never); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + it('persists organization_id and invalidates access cache when the user is a member', async () => { + const db = createApplyMetadataDb({ membershipRows: 1 }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['orgId', 'org_live'], + ['title', 'Hello'], + ]) + ); + + expect(db.membershipQueryCount()).toBe(1); + expect(db.updateSets).toEqual([ + expect.objectContaining({ + organization_id: 'org_live', + title: 'Hello', + }), + ]); + expect(getSessionAccessCacheDO).toHaveBeenCalledWith(env, { kiloUserId: 'usr_1' }); + expect(cacheRemove).toHaveBeenCalledWith('ses_1'); + expect(notifyUserSessionEvent).toHaveBeenCalledWith( + env, + 'usr_1', + expect.objectContaining({ type: 'session.updated' }), + undefined + ); + }); + + it('refuses unauthorized organization_id while persisting the rest of the batch', async () => { + const db = createApplyMetadataDb({ membershipRows: 0 }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const warnSpy = vi.mocked(console.warn); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['orgId', 'org_foreign'], + ['title', 'Kept title'], + ['gitUrl', 'https://github.com/acme/repo.git'], + ['status', 'busy'], + ]) + ); + + expect(db.membershipQueryCount()).toBe(1); + expect(db.updateSets).toHaveLength(1); + const written = db.updateSets[0] as Record; + expect(written).not.toHaveProperty('organization_id'); + expect(written.title).toBe('Kept title'); + expect(written.git_url).toBe('https://github.com/acme/repo'); + expect(written.status).toBe('busy'); + expect(written.status_updated_at).toEqual(expect.any(String)); + expect(warnSpy).toHaveBeenCalledWith( + 'Refusing unauthorized organization_id metadata write', + expect.objectContaining({ + kiloUserId: 'usr_1', + sessionId: 'ses_1', + organizationId: 'org_foreign', + }) + ); + }); + + it('does not treat a refused orgId-only batch as a scope change or session.updated', async () => { + const db = createApplyMetadataDb({ membershipRows: 0 }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges(env, 'usr_1', 'ses_1', new Map([['orgId', 'org_foreign']])); + + // Refused field is stripped; empty updates object skips the UPDATE entirely. + expect(db.applyUpdate).not.toHaveBeenCalled(); + expect(db.updateSets).toEqual([]); + expect(getSessionAccessCacheDO).not.toHaveBeenCalled(); + expect(cacheRemove).not.toHaveBeenCalled(); + expect(notifyUserSessionEvent).not.toHaveBeenCalled(); + }); + + it('still emits session.updated when a refused orgId is paired with parentId', async () => { + const db = createApplyMetadataDb({ membershipRows: 0, parentExists: true }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['orgId', 'org_foreign'], + ['parentId', 'ses_parent'], + ]) + ); + + expect(db.updateSets).toEqual([expect.objectContaining({ parent_session_id: 'ses_parent' })]); + expect(getSessionAccessCacheDO).not.toHaveBeenCalled(); + expect(notifyUserSessionEvent).toHaveBeenCalledWith( + env, + 'usr_1', + expect.objectContaining({ type: 'session.updated' }), + undefined + ); + }); + + it('refuses organization_id for a soft-deleted org while persisting the rest', async () => { + // Soft-deleted orgs yield no membership join row (deleted_at IS NULL filter). + const db = createApplyMetadataDb({ membershipRows: 0 }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const warnSpy = vi.mocked(console.warn); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['orgId', 'org_deleted'], + ['title', 'Still written'], + ['status', 'idle'], + ]) + ); + + const written = db.updateSets[0] as Record; + expect(written).not.toHaveProperty('organization_id'); + expect(written.title).toBe('Still written'); + expect(written.status).toBe('idle'); + expect(warnSpy).toHaveBeenCalledWith( + 'Refusing unauthorized organization_id metadata write', + expect.objectContaining({ organizationId: 'org_deleted' }) + ); + expect(getSessionAccessCacheDO).not.toHaveBeenCalled(); + }); + + it('performs zero membership queries when orgId is absent', async () => { + const db = createApplyMetadataDb(); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['title', 'No org'], + ['platform', 'cli'], + ['status', 'busy'], + ]) + ); + + expect(db.membershipQueryCount()).toBe(0); + expect(db.updateSets).toEqual([ + expect.objectContaining({ + title: 'No org', + created_on_platform: 'cli', + status: 'busy', + }), + ]); + const written = db.updateSets[0] as Record; + expect(written).not.toHaveProperty('organization_id'); + expect(getSessionAccessCacheDO).not.toHaveBeenCalled(); + }); + + it('clears organization_id on explicit null without a membership query', async () => { + const db = createApplyMetadataDb(); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges(env, 'usr_1', 'ses_1', new Map([['orgId', null]])); + + expect(db.membershipQueryCount()).toBe(0); + expect(db.updateSets).toEqual([expect.objectContaining({ organization_id: null })]); + expect(getSessionAccessCacheDO).toHaveBeenCalledWith(env, { kiloUserId: 'usr_1' }); + expect(cacheRemove).toHaveBeenCalledWith('ses_1'); + }); + + it('refuses a nonexistent org claim without aborting the rest of the batch', async () => { + // Nonexistent org looks like no membership row to the check; never reaches FK. + const db = createApplyMetadataDb({ membershipRows: 0 }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['orgId', '00000000-0000-4000-8000-000000000099'], + ['title', 'Survives'], + ['platform', 'cli'], + ['status', 'busy'], + ]) + ); + + const written = db.updateSets[0] as Record; + expect(written).not.toHaveProperty('organization_id'); + expect(written.title).toBe('Survives'); + expect(written.created_on_platform).toBe('cli'); + expect(written.status).toBe('busy'); + expect(db.applyUpdate).toHaveBeenCalled(); + }); +}); diff --git a/services/session-ingest/src/ingest/metadata.ts b/services/session-ingest/src/ingest/metadata.ts index b33a95c86a..9beef2536c 100644 --- a/services/session-ingest/src/ingest/metadata.ts +++ b/services/session-ingest/src/ingest/metadata.ts @@ -1,7 +1,7 @@ import { and, eq, inArray, sql } from 'drizzle-orm'; import { getWorkerDb } from '@kilocode/db/client'; import { cli_sessions_v2 } from '@kilocode/db/schema'; -import { normalizeGitUrl, withDORetry } from '@kilocode/worker-utils'; +import { hasOrganizationAccess, normalizeGitUrl, withDORetry } from '@kilocode/worker-utils'; import type { Env } from '../env'; import { getSessionAccessCacheDO } from '../dos/SessionAccessCacheDO'; @@ -65,13 +65,8 @@ export async function applyMetadataChanges( const parentSessionId = mergedChanges.has('parentId') ? (mergedChanges.get('parentId') ?? null) : undefined; - const changedNonStatus = - mergedChanges.has('title') || - mergedChanges.has('platform') || - mergedChanges.has('orgId') || - mergedChanges.has('gitUrl') || - mergedChanges.has('gitBranch') || - parentSessionId !== undefined; + /** True only when an organization_id write was actually applied (authorized claim or explicit null clear). */ + let organizationIdWriteApplied = false; const notification = await db.transaction(async tx => { const statusChange = @@ -96,6 +91,46 @@ export async function applyMetadataChanges( if (!statusChange) return null; + // Membership check only for non-null org claims; run on the same tx as the UPDATE. + // Residual: SessionIngestDO.writeIngestMetaIfChanged records the claimed orgId in DO + // SQLite and emits a change only when the value differs; after a refused write the DO + // believes the org is set while Postgres does not, so re-sending the same orgId later + // will not re-emit it. Desirable in the attack case; in the benign case (user genuinely + // joins the org afterwards) the session stays personal until the CLI sends a different + // value. Follow-up tracked in the PR body. + if (mergedChanges.has('orgId')) { + const organizationId = mergedChanges.get('orgId') ?? null; + if (organizationId !== null) { + const authorized = await hasOrganizationAccess(tx, { + kiloUserId, + organizationId, + }); + if (!authorized) { + console.warn('Refusing unauthorized organization_id metadata write', { + kiloUserId, + sessionId, + organizationId, + }); + delete updates.organization_id; + } else { + organizationIdWriteApplied = true; + } + } else { + organizationIdWriteApplied = true; + } + } + + // Gate only the orgId contribution: a refused-only orgId must not count as a + // non-status change (no phantom session.updated). Keep parentSessionId and every + // other non-org key exactly as before — do not derive this from `updates` alone. + const changedNonStatus = + mergedChanges.has('title') || + mergedChanges.has('platform') || + organizationIdWriteApplied || + mergedChanges.has('gitUrl') || + mergedChanges.has('gitBranch') || + parentSessionId !== undefined; + if (Object.keys(updates).length > 0) { await tx .update(cli_sessions_v2) @@ -179,7 +214,7 @@ export async function applyMetadataChanges( }; }); - if (mergedChanges.has('orgId')) { + if (organizationIdWriteApplied) { try { await withDORetry( () => getSessionAccessCacheDO(env, { kiloUserId }), diff --git a/services/session-ingest/src/queue-consumer.test.ts b/services/session-ingest/src/queue-consumer.test.ts index cab27e7fa5..7c26642d1d 100644 --- a/services/session-ingest/src/queue-consumer.test.ts +++ b/services/session-ingest/src/queue-consumer.test.ts @@ -945,10 +945,17 @@ describe('queue organization changes', () => { status: null, status_updated_at: null, }; - const selectResults: unknown[][] = [[{ session_id: sessionId }], [persistedSession]]; + // loadSession → membership join (authorized) → read-back after org write. + const selectResults: unknown[][] = [ + [{ session_id: sessionId }], + [{ id: 'mem_1' }], + [persistedSession], + ]; const selectResult = vi.fn(async () => selectResults.shift() ?? []); const select = { from: vi.fn(() => select), + // hasOrganizationAccess joins memberships → organizations (deleted_at IS NULL). + innerJoin: vi.fn(() => select), where: vi.fn(() => select), limit: vi.fn(() => select), for: vi.fn(() => select), From 2bce3aa3d435f97e6e63ff591aa5c42a44e7746f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 14:11:57 +0200 Subject: [PATCH 07/11] fix(cli-sessions-v2): hide never-ingested placeholder sessions from list and search POST /api/session creates bare placeholder rows before any user turn; if the CLI dies before its first debounced flush the row stays permanently title/status/cost NULL with created_on_platform 'unknown' and renders as Untitled session / UNKNOWN in the Agents list. Exclude rows where all four list columns are unwritten; any row with a title, a status, a known platform, or a persisted cost (including 0) still appears. --- .../routers/cli-sessions-v2-router.test.ts | 134 ++++++++++++++++++ .../web/src/routers/cli-sessions-v2-router.ts | 37 +++++ 2 files changed, 171 insertions(+) diff --git a/apps/web/src/routers/cli-sessions-v2-router.test.ts b/apps/web/src/routers/cli-sessions-v2-router.test.ts index 4b9b9bd5c9..50e77912b9 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.test.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.test.ts @@ -1713,4 +1713,138 @@ describe('cli-sessions-v2-router', () => { ]); }); }); + + describe('list / search hide never-ingested placeholders', () => { + // Bare POST /api/session placeholders: title/status/cost NULL and platform + // still at the column default 'unknown'. Content may still exist in DO/R2; + // the four-column conjunction is only a list/search visibility predicate. + const placeholderId = 'ses_hide_placeholder_bare_0001'; + const titledUnknownId = 'ses_hide_placeholder_titled_0001'; + const statusUnknownId = 'ses_hide_placeholder_status_0001'; + const costOnlyZeroId = 'ses_hide_placeholder_cost0_0001'; + const normalCliId = 'ses_hide_placeholder_cli_0001'; + const allSessionIds = [ + placeholderId, + titledUnknownId, + statusUnknownId, + costOnlyZeroId, + normalCliId, + ]; + + beforeEach(async () => { + const baseTime = Date.parse('2026-06-01T12:00:00.000Z'); + await db.insert(cli_sessions_v2).values([ + { + session_id: placeholderId, + kilo_user_id: regularUser.id, + // defaults: created_on_platform 'unknown', title/status/cost NULL + created_at: new Date(baseTime).toISOString(), + updated_at: new Date(baseTime).toISOString(), + }, + { + session_id: titledUnknownId, + kilo_user_id: regularUser.id, + created_on_platform: 'unknown', + title: 'titled but still unknown platform', + created_at: new Date(baseTime + 1000).toISOString(), + updated_at: new Date(baseTime + 1000).toISOString(), + }, + { + session_id: statusUnknownId, + kilo_user_id: regularUser.id, + created_on_platform: 'unknown', + status: 'running', + created_at: new Date(baseTime + 2000).toISOString(), + updated_at: new Date(baseTime + 2000).toISOString(), + }, + { + session_id: costOnlyZeroId, + kilo_user_id: regularUser.id, + created_on_platform: 'unknown', + // Metrics emission can persist 0 (writer clamps with Math.max(0, …)) + // while no metadata projection ever succeeded. + total_cost_microdollars: 0, + created_at: new Date(baseTime + 3000).toISOString(), + updated_at: new Date(baseTime + 3000).toISOString(), + }, + { + session_id: normalCliId, + kilo_user_id: regularUser.id, + created_on_platform: 'cli', + title: 'normal cli session', + status: 'completed', + created_at: new Date(baseTime + 4000).toISOString(), + updated_at: new Date(baseTime + 4000).toISOString(), + }, + ]); + }); + + afterEach(async () => { + await db.delete(cli_sessions_v2).where(inArray(cli_sessions_v2.session_id, allSessionIds)); + }); + + it('list omits bare placeholder rows', async () => { + const caller = await createCallerForUser(regularUser.id); + const result = await caller.cliSessionsV2.list({}); + const ids = result.cliSessions.map(session => session.session_id); + + expect(ids).not.toContain(placeholderId); + }); + + it('list returns a row with a title even when platform is still unknown', async () => { + const caller = await createCallerForUser(regularUser.id); + const result = await caller.cliSessionsV2.list({}); + const ids = result.cliSessions.map(session => session.session_id); + + expect(ids).toContain(titledUnknownId); + }); + + it('list returns a row with a status even when title is null and platform is unknown', async () => { + const caller = await createCallerForUser(regularUser.id); + const result = await caller.cliSessionsV2.list({}); + const ids = result.cliSessions.map(session => session.session_id); + + expect(ids).toContain(statusUnknownId); + }); + + it('list returns a row with only total_cost_microdollars set (including zero)', async () => { + const caller = await createCallerForUser(regularUser.id); + const result = await caller.cliSessionsV2.list({}); + const ids = result.cliSessions.map(session => session.session_id); + + expect(ids).toContain(costOnlyZeroId); + const costOnly = result.cliSessions.find(session => session.session_id === costOnlyZeroId); + expect(costOnly?.total_cost_microdollars).toBe(0); + }); + + it('list returns a normal cli session and keeps pagination stable with placeholders interleaved', async () => { + const caller = await createCallerForUser(regularUser.id); + // Fixtures are ordered by created_at; placeholders sit between visible + // rows. limit=2 over created_at should page only visible rows. + const page1 = await caller.cliSessionsV2.list({ limit: 2, orderBy: 'created_at' }); + const page1Ids = page1.cliSessions.map(session => session.session_id); + + expect(page1Ids).toEqual([normalCliId, costOnlyZeroId]); + expect(page1Ids).not.toContain(placeholderId); + expect(page1.nextCursor).not.toBeNull(); + + const page2 = await caller.cliSessionsV2.list({ + limit: 2, + orderBy: 'created_at', + cursor: page1.nextCursor!, + }); + const page2Ids = page2.cliSessions.map(session => session.session_id); + + expect(page2Ids).toEqual([statusUnknownId, titledUnknownId]); + expect(page2Ids).not.toContain(placeholderId); + }); + + it('search by exact session_id does not return a bare placeholder', async () => { + const caller = await createCallerForUser(regularUser.id); + const result = await caller.cliSessionsV2.search({ search_string: placeholderId }); + + expect(result.results.map(session => session.session_id)).not.toContain(placeholderId); + expect(result.total).toBe(0); + }); + }); }); diff --git a/apps/web/src/routers/cli-sessions-v2-router.ts b/apps/web/src/routers/cli-sessions-v2-router.ts index ec3c32d13f..e8d1856ef6 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.ts @@ -470,6 +470,41 @@ async function addOrganizationCondition( whereConditions.push(eq(cli_sessions_v2.organization_id, organizationId)); } +/** + * Hide never-ingested placeholder rows from list/search. + * + * POST /api/session creates bare placeholders (title/status/cost NULL, + * created_on_platform default 'unknown') before any user turn. Metadata and + * cost arrive later via ingest; if the client dies in that window the row + * stays permanently unwritten. + * + * All four columns unwritten proves only that no metadata projection ever + * succeeded and no metrics emission ever persisted a cost. It does not prove + * the row has no content — content lives in the DO and R2 and commits + * independently of the metadata projection. + * + * total_cost_microdollars is written by the alarm-driven metrics emission in + * SessionIngestDO.emitSessionMetrics (best-effort: stays NULL when the metric + * is non-finite or the UPDATE throws and is swallowed), not per flush — so + * presence proves the session reached a metrics emission and must be shown; + * absence proves nothing. + * + * Invariant: any row whose four list columns are all unwritten is hidden, + * regardless of whether the DO holds content. The predicate is the definition + * of what gets hidden; no Postgres-visible signal can do better on a + * paginated list query. + */ +function addHideUningestedPlaceholderCondition(whereConditions: SQL[]): void { + whereConditions.push( + sql`( + ${isNotNull(cli_sessions_v2.title)} + OR ${isNotNull(cli_sessions_v2.status)} + OR ${cli_sessions_v2.created_on_platform} != 'unknown' + OR ${isNotNull(cli_sessions_v2.total_cost_microdollars)} + )` + ); +} + function joinWithAnd(fragments: SQL[]): SQL { return sql.join(fragments, sql` AND `); } @@ -506,6 +541,7 @@ export const cliSessionsV2Router = createTRPCRouter({ await addOrganizationCondition(whereConditions, ctx, organizationId); addCreatedOnPlatformConditions(whereConditions, createdOnPlatform); addGitUrlConditions(whereConditions, gitUrl); + addHideUningestedPlaceholderCondition(whereConditions); if (cursor) { whereConditions.push(lt(orderColumn, cursor)); @@ -586,6 +622,7 @@ export const cliSessionsV2Router = createTRPCRouter({ await addOrganizationCondition(whereConditions, ctx, organizationId); addCreatedOnPlatformConditions(whereConditions, createdOnPlatform); addGitUrlConditions(whereConditions, gitUrl); + addHideUningestedPlaceholderCondition(whereConditions); if (!includeChildren) { whereConditions.push(isNull(cli_sessions_v2.parent_session_id)); From 43558b0f20e8f865e897d233990fe423bc969791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 14:19:20 +0200 Subject: [PATCH 08/11] chore: retrigger code review From 9e3fcba0789a78f782e6c1c0d84013e10a9eda94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 15:10:34 +0200 Subject: [PATCH 09/11] test(session-ingest): pin cost persist before O11Y metrics RPC Regression coverage for the guarantee that the Postgres total_cost_microdollars persist completes before the unguarded O11Y.ingestSessionMetrics RPC: drives alarm() with O11Y rejecting and asserts the update chain (connection, value, session/user filter) was awaited first. Marker records at await time via a thenable, so a build-now-await-later refactor also fails. --- .../src/dos/SessionIngestDO.test.ts | 247 ++++++++++++++++++ 1 file changed, 247 insertions(+) diff --git a/services/session-ingest/src/dos/SessionIngestDO.test.ts b/services/session-ingest/src/dos/SessionIngestDO.test.ts index bf9e7dd918..19e9a900ce 100644 --- a/services/session-ingest/src/dos/SessionIngestDO.test.ts +++ b/services/session-ingest/src/dos/SessionIngestDO.test.ts @@ -5,6 +5,10 @@ const drizzleMocks = vi.hoisted(() => ({ migrate: vi.fn(), })); +const dbClientMocks = vi.hoisted(() => ({ + getWorkerDb: vi.fn(), +})); + vi.mock('cloudflare:workers', () => ({ DurableObject: class DurableObject { ctx: unknown; @@ -24,6 +28,10 @@ vi.mock('drizzle-orm/durable-sqlite/migrator', () => ({ migrate: drizzleMocks.migrate, })); +vi.mock('@kilocode/db/client', () => ({ + getWorkerDb: dbClientMocks.getWorkerDb, +})); + import { SessionIngestDO, ingestOrderCursor } from './SessionIngestDO'; describe('SessionIngestDO ingest ordering', () => { @@ -374,3 +382,242 @@ describe('SessionIngestDO session-ready push', () => { expect(sendSessionReadyNotification).not.toHaveBeenCalled(); }); }); + +describe('SessionIngestDO emitSessionMetrics cost persist ordering', () => { + /** + * Pins: Postgres total_cost_microdollars persist runs before the unguarded + * O11Y.ingestSessionMetrics RPC. An O11Y rejection must not skip the persist. + */ + function makeAlarmHarness(options: { + totalCostDollars: number; + o11yImpl: () => Promise; + }) { + const operations: string[] = []; + const meta = new Map([ + ['kiloUserId', 'usr_cost'], + ['sessionId', 'ses_cost'], + ['closeReason', 'completed'], + ['ingestVersion', '3'], + ]); + + const itemData = JSON.stringify({ + role: 'assistant', + time: { created: 1000 }, + tokens: { input: 100, output: 50, reasoning: 0, cache: { read: 0, write: 0 } }, + cost: options.totalCostDollars, + }); + const ingestItemRows = [{ item_type: 'message', item_data: itemData }]; + + type EqCondition = { queryChunks?: unknown[] }; + /** Walk nested drizzle queryChunks (eq / and) and collect string Param values. */ + const collectBoundStringParams = (condition: unknown, out: string[] = []): string[] => { + const chunks = (condition as EqCondition | undefined)?.queryChunks ?? []; + for (const chunk of chunks) { + if (chunk == null || typeof chunk !== 'object') continue; + const value = (chunk as { value?: unknown }).value; + if (typeof value === 'string') { + out.push(value); + continue; + } + // Nested SQL (e.g. and(eq(...), eq(...))) embeds child conditions as chunks. + if ('queryChunks' in (chunk as object)) { + collectBoundStringParams(chunk, out); + } + } + return out; + }; + const extractEqValue = (condition: unknown): string | undefined => { + return collectBoundStringParams(condition)[0]; + }; + + const selectQuery = { + from: vi.fn(() => selectQuery), + where: vi.fn((condition: unknown) => { + // Stash eq-bound key for .get(); .all() paths ignore it. + (selectQuery as { _key?: string })._key = extractEqValue(condition); + return selectQuery; + }), + orderBy: vi.fn(() => selectQuery), + get: vi.fn(() => { + const key = (selectQuery as { _key?: string })._key; + if (key === 'metricsEmitted') { + const value = meta.get('metricsEmitted'); + return value === undefined ? undefined : { value }; + } + if (key === 'model') { + return undefined; + } + // alarm() loads meta via select().from().where(inArray(...)).all() + // — handled by all() below. get() for other keys: + if (key !== undefined && meta.has(key)) { + return { value: meta.get(key) }; + } + return undefined; + }), + all: vi.fn(() => { + // alarm meta load: returns rows with key/value + // emitSessionMetrics item load: returns item_type/item_data rows + // Distinguish by whether the last where bound a single eq key used for items. + // Simpler: track call site via select columns shape. + return (selectQuery as { _allKind?: 'meta' | 'items' })._allKind === 'items' + ? ingestItemRows + : [...meta.entries()].map(([key, value]) => ({ key, value })); + }), + }; + + const originalSelect = vi.fn((columns?: unknown) => { + if ( + columns && + typeof columns === 'object' && + 'item_type' in (columns as Record) + ) { + (selectQuery as { _allKind?: 'meta' | 'items' })._allKind = 'items'; + } else if ( + columns && + typeof columns === 'object' && + 'item_data' in (columns as Record) && + !('item_type' in (columns as Record)) + ) { + // model lookup: select({ item_data }).from().where(eq item_id 'model').get() + (selectQuery as { _allKind?: 'meta' | 'items' })._allKind = undefined; + } else if ( + columns && + typeof columns === 'object' && + 'value' in (columns as Record) + ) { + // metricsEmitted check + (selectQuery as { _allKind?: 'meta' | 'items' })._allKind = undefined; + } else { + // bare select() for alarm meta + (selectQuery as { _allKind?: 'meta' | 'items' })._allKind = 'meta'; + } + return selectQuery; + }); + + const db = { + select: originalSelect, + insert: vi.fn(() => ({ + values: vi.fn((values: { key?: string; value?: string | null }) => ({ + onConflictDoUpdate: vi.fn(() => ({ + run: vi.fn(() => { + if (values.key !== undefined) { + meta.set(values.key, values.value ?? null); + operations.push(`meta:${values.key}:${values.value}`); + } + }), + })), + })), + })), + delete: vi.fn(() => ({ where: vi.fn(() => ({ run: vi.fn() })) })), + }; + drizzleMocks.db = db; + + let persistedMicrodollars: number | undefined; + let pgWhereCondition: unknown; + let pgWhereBoundParams: string[] = []; + /** + * Record persist only when the drizzle chain is AWAITED (via .then), not when + * .where() is merely invoked. A build-now-await-later refactor must fail this test. + */ + const pgWhere = vi.fn((condition: unknown) => { + pgWhereCondition = condition; + pgWhereBoundParams = collectBoundStringParams(condition); + return { + then( + onFulfilled?: ((value: unknown) => unknown) | null, + onRejected?: ((reason: unknown) => unknown) | null + ) { + operations.push('persist:total_cost_microdollars'); + return Promise.resolve(undefined).then(onFulfilled, onRejected); + }, + }; + }); + const pgSet = vi.fn((set: { total_cost_microdollars?: number }) => { + persistedMicrodollars = set.total_cost_microdollars; + return { where: pgWhere }; + }); + const pgUpdate = vi.fn(() => ({ set: pgSet })); + dbClientMocks.getWorkerDb.mockReset(); + dbClientMocks.getWorkerDb.mockReturnValue({ update: pgUpdate }); + + const ingestSessionMetrics = vi.fn(async () => { + operations.push('o11y:ingestSessionMetrics'); + return options.o11yImpl(); + }); + + const deleteAlarm = vi.fn(async () => { + operations.push('deleteAlarm'); + }); + const state = { + storage: { setAlarm: vi.fn(), deleteAlarm }, + blockConcurrencyWhile: vi.fn((fn: () => void) => fn()), + } as unknown as DurableObjectState; + + const env = { + SESSION_INGEST_R2: { delete: vi.fn() }, + HYPERDRIVE: { connectionString: 'postgres://test' }, + O11Y: { ingestSessionMetrics }, + } as never; + + return { + durableObject: new SessionIngestDO(state, env), + operations, + ingestSessionMetrics, + pgSet, + pgWhere, + getWorkerDb: dbClientMocks.getWorkerDb, + get persistedMicrodollars() { + return persistedMicrodollars; + }, + get pgWhereCondition() { + return pgWhereCondition; + }, + get pgWhereBoundParams() { + return pgWhereBoundParams; + }, + meta, + deleteAlarm, + }; + } + + it('persists total_cost_microdollars before O11Y when O11Y rejects', async () => { + const o11yError = new Error('o11y unavailable'); + const harness = makeAlarmHarness({ + totalCostDollars: 0.15, + o11yImpl: async () => { + throw o11yError; + }, + }); + + await expect(harness.durableObject.alarm()).rejects.toThrow('o11y unavailable'); + + expect(harness.getWorkerDb).toHaveBeenCalledWith('postgres://test'); + expect(harness.pgSet).toHaveBeenCalledWith({ total_cost_microdollars: 150_000 }); + expect(harness.persistedMicrodollars).toBe(150_000); + expect(harness.pgWhere).toHaveBeenCalledTimes(1); + // where() must bind both session_id and kilo_user_id (and(...) nests eq chunks). + expect(harness.pgWhereBoundParams).toEqual(expect.arrayContaining(['ses_cost', 'usr_cost'])); + expect(harness.ingestSessionMetrics).toHaveBeenCalledTimes(1); + expect(harness.ingestSessionMetrics).toHaveBeenCalledWith( + expect.objectContaining({ + kiloUserId: 'usr_cost', + sessionId: 'ses_cost', + ingestVersion: 3, + totalCost: 0.15, + terminationReason: 'completed', + }) + ); + + // Ordering: persist must precede the unguarded O11Y RPC. + const persistIdx = harness.operations.indexOf('persist:total_cost_microdollars'); + const o11yIdx = harness.operations.indexOf('o11y:ingestSessionMetrics'); + expect(persistIdx).toBeGreaterThanOrEqual(0); + expect(o11yIdx).toBeGreaterThanOrEqual(0); + expect(persistIdx).toBeLessThan(o11yIdx); + + // Rejection propagates out of alarm(); metricsEmitted must not be marked. + expect(harness.operations).not.toContain('meta:metricsEmitted:true'); + expect(harness.operations).not.toContain('deleteAlarm'); + expect(harness.meta.get('metricsEmitted')).toBeUndefined(); + }); +}); From 82364623280369d985c760f2e912f52daa8d01af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 15:32:08 +0200 Subject: [PATCH 10/11] style(session-ingest): oxfmt the DO ordering test --- services/session-ingest/src/dos/SessionIngestDO.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/services/session-ingest/src/dos/SessionIngestDO.test.ts b/services/session-ingest/src/dos/SessionIngestDO.test.ts index 19e9a900ce..9e169a8b54 100644 --- a/services/session-ingest/src/dos/SessionIngestDO.test.ts +++ b/services/session-ingest/src/dos/SessionIngestDO.test.ts @@ -388,10 +388,7 @@ describe('SessionIngestDO emitSessionMetrics cost persist ordering', () => { * Pins: Postgres total_cost_microdollars persist runs before the unguarded * O11Y.ingestSessionMetrics RPC. An O11Y rejection must not skip the persist. */ - function makeAlarmHarness(options: { - totalCostDollars: number; - o11yImpl: () => Promise; - }) { + function makeAlarmHarness(options: { totalCostDollars: number; o11yImpl: () => Promise }) { const operations: string[] = []; const meta = new Map([ ['kiloUserId', 'usr_cost'], From ef306dc122debaaa8b12fd48f584d7c9729b02d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 26 Jul 2026 17:14:03 +0200 Subject: [PATCH 11/11] chore: retrigger code review