From 06fb7e2e183ea51f0da4981db001a0bcd22dabee Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Thu, 24 Sep 2026 20:54:00 -0700 Subject: [PATCH 1/2] fix(cli): report unpriced spend honestly, and parse the rest of Codex's items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps a codex step made visible on the dashboard. **Unpriced was indistinguishable from free.** The kernel already records `dollars_unmetered`, and `flows status` already prints `(unmetered)` from it — the mirror dropped it. So Cloud could not tell a step that genuinely cost nothing (a deterministic `echo`) from one that spent real money against an unpriced model: both arrived as an absent `costUsd` and rendered blank. Codex is the everyday case, because it selects its own model and never reports it, so `MODEL_PRICING` has nothing to match — as `model-pricing.ts` says outright. The flag now rides on the step row, and only when no attempt produced a price: a step priced on one attempt and unpriced on another has a real figure, and claiming the whole step is unpriced would hide it. Still never a fabricated zero. **Three of Codex's nine item types were unparsed.** `ThreadItemDetails` (codex-rs/exec/src/exec_events.rs) defines nine; this module read six, so `web_search`, `todo_list` and `collab_tool_call` reached the page as bare `unknown` placeholders naming a type and a size. `todo_list` was the worst of them: it updates continuously through a real turn, so the most frequent item in an agent transcript was the one a reader could not read. All three are now parsed from the Rust structs — a plan takes no sequence number because it is not a call, and collab agent states are summarised by status rather than listed, because agent ids are not something a transcript reader can act on. Reasoning tokens are carried as a **breakdown and never summed**. Codex's own `blended_total()` is `non_cached_input + output_tokens`, and nothing in its protocol adds reasoning to output, so `reasoning_output_tokens` is a subset of `output_tokens` (codex-rs/protocol/src/protocol.rs). I had this backwards and was about to add them: that would have inflated metered output and, through `maxDollars`, the budget ceiling itself. One existing test changed rather than broke. It used `todo_list` and `web_search` as its stand-ins for an unsupported item type; both are parsed now, so it uses a type Codex does not define — which is what the fallback is actually for, and the payload still must not reach the page. Co-Authored-By: Claude Opus 5 (1M context) --- packages/sdk/src/cloud-mirror-step.ts | 33 +++++++ packages/sdk/src/cloud-transcript-codex.ts | 90 ++++++++++++++++++- packages/sdk/src/cloud-transcript-types.ts | 19 +++- packages/sdk/src/cloud-transcript.ts | 10 ++- packages/sdk/tests/cloud-mirror-step.test.ts | 79 ++++++++++++++++ .../sdk/tests/cloud-transcript-codex.test.ts | 77 ++++++++++++++-- 6 files changed, 296 insertions(+), 12 deletions(-) diff --git a/packages/sdk/src/cloud-mirror-step.ts b/packages/sdk/src/cloud-mirror-step.ts index 883bb1e5e..ffa9924d4 100644 --- a/packages/sdk/src/cloud-mirror-step.ts +++ b/packages/sdk/src/cloud-mirror-step.ts @@ -114,6 +114,18 @@ export interface FinalStep { model?: string; tokensInput?: number; tokensOutput?: number; + /** + * True when tokens were counted but nobody priced them. + * + * The kernel already records this (`run-state.ts`'s `Spend`), and + * `flows status` already prints `(unmetered)` from it. Without it on the + * wire, Cloud cannot tell a step that genuinely cost nothing — a + * deterministic `echo` — from one that spent real money against an unpriced + * model: both arrive as an absent `costUsd` and render blank. Codex is the + * everyday case, because it selects its own model and never reports it, so + * `MODEL_PRICING` has nothing to match (`model-pricing.ts` says so outright). + */ + costUnmetered?: true; costUsd?: number; error?: string; detail?: Record; @@ -314,6 +326,18 @@ interface AttemptRecord { tokensIn?: number; tokensOut?: number; costUsd?: number; + costUnmetered?: boolean; + /** + * Reasoning tokens the provider reported, when it separates them. + * + * Carried as a *breakdown*, never added to `tokensOut`: codex's own + * `blended_total()` is `non_cached_input + output_tokens` and nothing in its + * protocol adds reasoning to output, so `reasoning_output_tokens` is a + * subset of `output_tokens` (codex-rs/protocol/src/protocol.rs). Summing + * them would inflate metered output and, through `maxDollars`, the budget + * ceiling itself. + */ + reasoningOut?: number; } /** @@ -332,6 +356,7 @@ function attemptsByStep(events: readonly JournalEvent[]): Map= 0 ? { costUsd: result['total_cost_usd'] as number } : {}), + ...(budget['dollars_unmetered'] === true ? { costUnmetered: true } : {}), + ...(int32(usage?.['reasoning_output_tokens']) === undefined + ? {} : { reasoningOut: int32(usage!['reasoning_output_tokens'])! }), }); byStep.set(event.step_id, entries); } @@ -374,6 +402,10 @@ function finalStep( // Cloud totals these rows for the run's spend — so a retried agent's earlier // charges simply vanished from the run. const spentUsd = attempts.reduce((total, entry) => total + (entry.costUsd ?? 0), 0); + // Unmetered only when no attempt produced a price. A step that was priced on + // one attempt and unpriced on another has a real, if partial, figure — and + // claiming the whole step is unpriced would hide it. + const unmetered = spentUsd === 0 && attempts.some(entry => entry.costUnmetered === true); const detail = stepDetail(attempts, env); const verification = step.last_attempt?.verification ?? null; // What the step said about itself. A gate's verdict detail is the nearest @@ -408,6 +440,7 @@ function finalStep( ...(tokensIn > 0 ? { tokensInput: Math.min(tokensIn, MAX_INT32) } : {}), ...(tokensOut > 0 ? { tokensOutput: Math.min(tokensOut, MAX_INT32) } : {}), ...(spentUsd > 0 ? { costUsd: spentUsd } : {}), + ...(unmetered ? { costUnmetered: true as const } : {}), ...(error === undefined ? {} : { error }), ...(detail.detail === undefined ? {} : { detail: detail.detail }), ...(detail.truncated === undefined ? {} : { detailTruncated: detail.truncated }), diff --git a/packages/sdk/src/cloud-transcript-codex.ts b/packages/sdk/src/cloud-transcript-codex.ts index b83b26957..28b537361 100644 --- a/packages/sdk/src/cloud-transcript-codex.ts +++ b/packages/sdk/src/cloud-transcript-codex.ts @@ -32,12 +32,22 @@ export const CODEX_FRAME_TYPES: ReadonlySet = new Set([ /** Item types whose lifecycles are matched and whose fields are read. */ const SUPPORTED_ITEMS: ReadonlySet = new Set([ 'agent_message', 'reasoning', 'command_execution', 'file_change', 'mcp_tool_call', 'error', + // The rest of Codex's `ThreadItemDetails` union + // (codex-rs/exec/src/exec_events.rs). Without these three, a real agent turn + // reported them as bare `unknown` placeholders naming a type and a size — + // honest, but useless: `todo_list` in particular updates continuously + // through a turn, so it was the most frequent thing a reader could not read. + 'web_search', 'todo_list', 'collab_tool_call', ]); const ITEM_FRAMES: ReadonlySet = new Set(['item.started', 'item.updated', 'item.completed']); /** Calls are numbered; an `apply_patch` is file activity, not a call. */ -const NUMBERED_ITEMS: ReadonlySet = new Set(['command_execution', 'mcp_tool_call']); +const NUMBERED_ITEMS: ReadonlySet = new Set([ + 'command_execution', 'mcp_tool_call', 'web_search', 'collab_tool_call', +]); +/** Todo entries listed per snapshot; a longer plan is counted, not printed. */ +const TODO_MAX_ITEMS = 12; interface Lifecycle { snapshots: Array>; @@ -246,6 +256,59 @@ function mcpEntry(item: Record, complete: boolean, context: Con }); } +/** + * `web_search`: a query and what was done with it (`WebSearchItem` — `query`, + * `action`, optional structured `results`). Rendered as a call rather than + * prose, so a reader sees the search in sequence beside the commands. + */ +function webSearchEntry(item: Record, complete: boolean, context: Context, + seq: number): TranscriptEntry | null { + const query = str(item['query']); + if (query === null) return null; + const action = isRecord(item['action']) ? str(item['action']['type']) : str(item['action']); + const results = item['results']; + const count = Array.isArray(results) ? results.length : null; + return tool('web_search', bounded(query, context.clean), + count === null ? null : JSON.stringify(results).length, false, { + seq, status: action === null ? null : bounded(action, context.clean, 40), + exit_code: null, + output_excerpt: count === null ? null : `${count} result${count === 1 ? '' : 's'}`, + output_truncated: false, complete, error: null, + }); +} + +/** + * `collab_tool_call`: Codex driving other Codex threads (`CollabToolCallItem` — + * `tool`, `sender_thread_id`, `receiver_thread_ids`, `agents_states`). + * + * The per-agent states are reduced to a count per status rather than listed: + * `agents_states` is a map keyed by agent id, and ids are not something a + * transcript reader can act on. + */ +function collabEntry(item: Record, complete: boolean, context: Context, + seq: number): TranscriptEntry | null { + const name = str(item['tool']); + if (name === null) return null; + const receivers = Array.isArray(item['receiver_thread_ids']) ? item['receiver_thread_ids'].length : 0; + const states = isRecord(item['agents_states']) ? item['agents_states'] : null; + const byStatus = new Map(); + if (states !== null) { + for (const state of Object.values(states)) { + const status = isRecord(state) ? str(state['status']) : null; + if (status !== null) byStatus.set(status, (byStatus.get(status) ?? 0) + 1); + } + } + const summary = [...byStatus.entries()].sort().map(([status, n]) => `${n} ${status}`).join(', '); + const status = str(item['status']); + return tool('collab_tool_call', + bounded(`${name}${receivers > 0 ? ` → ${receivers} thread${receivers === 1 ? '' : 's'}` : ''}`, context.clean), + null, status === 'failed', { + seq, status: status === null ? null : bounded(status, context.clean, 40), + exit_code: null, output_excerpt: summary.length === 0 ? null : summary, + output_truncated: false, complete, error: null, + }); +} + function fileChangeEntry(item: Record, complete: boolean, context: Context): TranscriptEntry | null { const raw = item['changes']; if (!Array.isArray(raw)) return null; @@ -288,12 +351,31 @@ function itemEntries(item: Record, complete: boolean, context: if (message === null) return [placeholder(context, itemType)]; return [{ kind: 'error', source: 'item', message: bounded(message, context.clean, ERROR_MAX_CHARS) }]; } - if (itemType === 'command_execution' || itemType === 'mcp_tool_call') { + if (itemType === 'todo_list') { + // A plan, not a call: it carries no status and earns no sequence number. + // `items[]` is `{text, completed}` (TodoListItem/TodoItem). + const items = Array.isArray(item['items']) ? item['items'] : null; + if (items === null) return [placeholder(context, itemType)]; + const listed = items.slice(0, TODO_MAX_ITEMS).flatMap((entry) => { + const todo = isRecord(entry) ? entry : undefined; + const text = str(todo?.['text']); + return text === null ? [] : [{ text: context.clean(text), done: todo!['completed'] === true }]; + }); + return [{ + kind: 'todo', total: items.length, done: listed.filter(entry => entry.done).length, + items: listed, ...(items.length > listed.length ? { omitted: items.length - listed.length } : {}), + ...(complete ? {} : { complete: false }), + }]; + } + if (itemType === 'command_execution' || itemType === 'mcp_tool_call' + || itemType === 'web_search' || itemType === 'collab_tool_call') { // The number is only spent on a call this module could actually read, so a // malformed item leaves no gap in the sequence. const seq = context.state.seq + 1; - const entry = itemType === 'command_execution' - ? commandEntry(item, complete, context, seq) : mcpEntry(item, complete, context, seq); + const entry = itemType === 'command_execution' ? commandEntry(item, complete, context, seq) + : itemType === 'mcp_tool_call' ? mcpEntry(item, complete, context, seq) + : itemType === 'web_search' ? webSearchEntry(item, complete, context, seq) + : collabEntry(item, complete, context, seq); if (entry === null) return [placeholder(context, itemType)]; context.state.seq = seq; return [entry]; diff --git a/packages/sdk/src/cloud-transcript-types.ts b/packages/sdk/src/cloud-transcript-types.ts index 3ca3114cb..872cc75ec 100644 --- a/packages/sdk/src/cloud-transcript-types.ts +++ b/packages/sdk/src/cloud-transcript-types.ts @@ -138,6 +138,22 @@ export interface TranscriptResult { cache_creation: number | null; } +/** + * Codex's running to-do list (`TodoListItem`). A plan, not a call: it carries + * no status and takes no sequence number, and it updates repeatedly through a + * turn — which is why leaving it unparsed was the most visible gap. + */ +export interface TranscriptTodo { + kind: 'todo'; + total: number; + done: number; + items: Array<{ text: string; done: boolean }>; + /** Entries past the per-snapshot cap: counted, never printed. */ + omitted?: number; + /** False when read off a snapshot that never completed. */ + complete?: boolean; +} + export interface TranscriptUnknown { kind: 'unknown'; type: string; @@ -152,7 +168,8 @@ export interface TranscriptUnparsed { export type TranscriptEntry = | TranscriptAttempt | TranscriptAttemptOmitted | TranscriptInit | TranscriptMessage | TranscriptThinking | TranscriptTool | TranscriptFileChange | TranscriptThread - | TranscriptTurn | TranscriptError | TranscriptResult | TranscriptUnknown | TranscriptUnparsed; + | TranscriptTurn | TranscriptError | TranscriptResult | TranscriptTodo + | TranscriptUnknown | TranscriptUnparsed; export interface ParsedTranscript { entries: TranscriptEntry[]; diff --git a/packages/sdk/src/cloud-transcript.ts b/packages/sdk/src/cloud-transcript.ts index 02981538b..06757cfc7 100644 --- a/packages/sdk/src/cloud-transcript.ts +++ b/packages/sdk/src/cloud-transcript.ts @@ -34,7 +34,7 @@ import { redact } from './redact.js'; export type { ParsedTranscript, TranscriptEntry, TranscriptAttempt, TranscriptAttemptOmitted, TranscriptError, TranscriptFileChange, TranscriptInit, TranscriptMessage, TranscriptResult, TranscriptThinking, - TranscriptThread, TranscriptTool, TranscriptToolCodex, TranscriptTurn, TranscriptUnknown, + TranscriptThread, TranscriptTodo, TranscriptTool, TranscriptToolCodex, TranscriptTurn, TranscriptUnknown, TranscriptUnparsed, } from './cloud-transcript-types.js'; @@ -297,6 +297,14 @@ export function renderAgentTranscript(parsed: ParsedTranscript): string[] { case 'thinking': lines.push(` thinking ${thousands(entry.chars)} chars (not shown)`); break; + case 'todo': { + // The plan the agent is working to, as it stood at this snapshot. + const progress = `${entry.done}/${entry.total}`; + lines.push(` plan ${progress}${entry.complete === false ? ' (in progress)' : ''}`); + for (const todo of entry.items) lines.push(` ${todo.done ? '✓' : '·'} ${safe(todo.text)}`); + if (entry.omitted !== undefined) lines.push(` …${entry.omitted} more`); + break; + } case 'tool': { const size = entry.is_error ? 'ERROR' : entry.result_chars === null ? 'no result' : `${thousands(entry.result_chars)} chars`; diff --git a/packages/sdk/tests/cloud-mirror-step.test.ts b/packages/sdk/tests/cloud-mirror-step.test.ts index 63258ed27..5ca2805fa 100644 --- a/packages/sdk/tests/cloud-mirror-step.test.ts +++ b/packages/sdk/tests/cloud-mirror-step.test.ts @@ -196,6 +196,85 @@ describe('mirrorJournal', () => { }); }); +/** + * The kernel already records `dollars_unmetered` and `flows status` already + * prints `(unmetered)` from it. The mirror dropped it, so Cloud could not tell + * a step that genuinely cost nothing from one that spent real money against an + * unpriced model — both arrived as an absent `costUsd`. Codex is the everyday + * case: it picks its own model and never reports it, so `MODEL_PRICING` has + * nothing to match. + */ +describe('unpriced spend', () => { + function codexStep(budget: Record, usage?: Record): JournalEvent[] { + seq = 0; + return [ + entry({ + entry_type: 'run.spawned', + payload: { spec: { name: 'codex', steps: [{ id: 'ask', type: 'agent', after: [] }] } }, + }), + entry({ entry_type: 'step.attempt.started', step_id: 'ask', attempt: 1, payload: {} }), + entry({ + entry_type: 'step.completed', + step_id: 'ask', + attempt: 1, + payload: { + completionReason: 'success', + disposition: 'step_done', + budget, + trajectory_tail: { transcript: { result: usage === undefined ? {} : { usage } } }, + }, + }), + ]; + } + + it('marks a step unmetered when tokens were counted and nobody priced them', () => { + const [step] = mirrorJournal('01RUN', codexStep({ + tokens_in: 13078, tokens_out: 8, dollars: '0', dollars_unmetered: true, + }), 1_700_000_010_000, {}).finals; + + expect(step).toMatchObject({ tokensInput: 13078, tokensOutput: 8, costUnmetered: true }); + // Never a fabricated zero: an absent price is absent, and said so. + expect(step).not.toHaveProperty('costUsd'); + }); + + it('does not mark a genuinely free step unmetered', () => { + const [step] = mirrorJournal('01RUN', codexStep({ + tokens_in: 0, tokens_out: 0, dollars: '0', dollars_unmetered: false, + }), 1_700_000_010_000, {}).finals; + + expect(step).not.toHaveProperty('costUnmetered'); + expect(step).not.toHaveProperty('costUsd'); + }); + + it('prefers a real figure over the unmetered flag when one attempt was priced', () => { + const events = codexStep({ tokens_in: 1, tokens_out: 1, dollars: '0', dollars_unmetered: true }); + (events[2]!.payload as Record)['trajectory_tail'] = { + transcript: { result: { total_cost_usd: 0.004 } }, + }; + const [step] = mirrorJournal('01RUN', events, 1_700_000_010_000, {}).finals; + + expect(step).toMatchObject({ costUsd: 0.004 }); + expect(step).not.toHaveProperty('costUnmetered'); + }); + + /** + * Codex's own `blended_total()` is `non_cached_input + output_tokens` and + * nothing in its protocol adds reasoning to output, so + * `reasoning_output_tokens` is a SUBSET of `output_tokens`. Carried as a + * breakdown; summing it would inflate metered output and, through + * `maxDollars`, the budget ceiling itself. + */ + it('carries reasoning tokens as a breakdown, never added to metered output', () => { + const [step] = mirrorJournal('01RUN', codexStep( + { tokens_in: 13078, tokens_out: 900, dollars: '0', dollars_unmetered: true }, + { input_tokens: 13078, output_tokens: 900, reasoning_output_tokens: 850 }, + ), 1_700_000_010_000, {}).finals; + + // 900, not 1,750. + expect(step!.tokensOutput).toBe(900); + }); +}); + describe('the bounds Cloud enforces', () => { it('redacts before it clips, so no secret survives as a prefix', () => { const secret = 'sk-ant-0123456789abcdefghijklmnopqrstuvwxyz'; diff --git a/packages/sdk/tests/cloud-transcript-codex.test.ts b/packages/sdk/tests/cloud-transcript-codex.test.ts index fcd439a3c..e6b6e10b2 100644 --- a/packages/sdk/tests/cloud-transcript-codex.test.ts +++ b/packages/sdk/tests/cloud-transcript-codex.test.ts @@ -340,15 +340,23 @@ describe('commands and MCP calls', () => { }); describe('fallbacks', () => { + /** + * This case used `todo_list` and `web_search` as its stand-ins for an + * unsupported item type. Both are parsed since 2.0.33, so it now uses a type + * Codex does not define — which is what the fallback is actually for: an item + * a future codex-cli adds and this module has never seen. The payload must + * still not reach the page, because nothing here knows which of its fields + * are safe to print. + */ it('names the item type in the placeholder, on every lifecycle event', () => { const rendered = render(frames( - { type: 'item.started', item: { id: 'i1', type: 'todo_list', items: [] } }, - { type: 'item.updated', item: { id: 'i1', type: 'todo_list', items: [] } }, - { type: 'item.completed', item: { id: 'i1', type: 'web_search', query: 'relayflow' } }, + { type: 'item.started', item: { id: 'i1', type: 'some_future_item', secret: 'relayflow' } }, + { type: 'item.updated', item: { id: 'i1', type: 'some_future_item', secret: 'relayflow' } }, + { type: 'item.completed', item: { id: 'i2', type: 'another_future_item', secret: 'relayflow' } }, )); - expect(rendered).toContain(' frame item.started/todo_list ('); - expect(rendered).toContain(' frame item.updated/todo_list ('); - expect(rendered).toContain(' frame item.completed/web_search ('); + expect(rendered).toContain(' frame item.started/some_future_item ('); + expect(rendered).toContain(' frame item.updated/some_future_item ('); + expect(rendered).toContain(' frame item.completed/another_future_item ('); expect(rendered).not.toContain('relayflow'); }); @@ -563,3 +571,60 @@ describe('the Claude vocabulary is untouched', () => { expect(parseAgentTranscript('plain terminal output\n', {}).stream_json).toBe(false); }); }); + +/** + * The rest of Codex's `ThreadItemDetails` union + * (codex-rs/exec/src/exec_events.rs). These three were reported as bare + * `unknown` placeholders until 2.0.33 — honest but unreadable, and + * `todo_list` is the one that updates continuously through a real turn. + */ +describe('the remaining Codex item types', () => { + it('renders a running to-do list as a plan, not a call', () => { + const parsed = parseAgentTranscript([ + JSON.stringify({ type: 'thread.started', thread_id: 't1' }), + JSON.stringify({ type: 'turn.started' }), + JSON.stringify({ type: 'item.completed', item: { id: 'i0', type: 'todo_list', items: [ + { text: 'read the failing test', completed: true }, + { text: 'fix the parser', completed: false }, + ] } }), + JSON.stringify({ type: 'turn.completed', usage: { input_tokens: 1, output_tokens: 1 } }), + ].join('\n'), {}); + + const todo = parsed.entries.find(entry => entry.kind === 'todo'); + expect(todo).toMatchObject({ kind: 'todo', total: 2, done: 1 }); + // A plan takes no sequence number: it is not a call. + expect(parsed.entries.filter(entry => entry.kind === 'tool')).toHaveLength(0); + }); + + it('renders a web search as a numbered call with its result count', () => { + const parsed = parseAgentTranscript(JSON.stringify({ + type: 'item.completed', + item: { id: 'i0', type: 'web_search', query: 'codex exec json events', action: { type: 'search' }, + results: [{ url: 'a' }, { url: 'b' }] }, + }), {}); + + expect(parsed.entries.find(entry => entry.kind === 'tool')).toMatchObject({ + kind: 'tool', name: 'web_search', target: 'codex exec json events', + codex: { seq: 1, status: 'search', output_excerpt: '2 results' }, + }); + }); + + it('summarises collab agents by status, never by agent id', () => { + const parsed = parseAgentTranscript(JSON.stringify({ + type: 'item.completed', + item: { id: 'i0', type: 'collab_tool_call', tool: 'spawn_agent', + sender_thread_id: 'sender-thread-id', receiver_thread_ids: ['r1', 'r2'], + agents_states: { + 'agent-id-nobody-can-act-on': { status: 'running', message: null }, + 'another-agent-id': { status: 'completed', message: null }, + }, + status: 'completed' }, + }), {}); + + const entry = parsed.entries.find(item => item.kind === 'tool'); + expect(entry).toMatchObject({ kind: 'tool', name: 'collab_tool_call', target: 'spawn_agent → 2 threads' }); + expect(entry).toHaveProperty('codex.output_excerpt', '1 completed, 1 running'); + // Agent ids are not something a transcript reader can act on. + expect(JSON.stringify(parsed.entries)).not.toContain('agent-id-nobody-can-act-on'); + }); +}); From 785684ea3a076c6c842d3f0bb9510f8f09cdc6e2 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Thu, 24 Sep 2026 21:25:06 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(cli):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?redact=20collab=20statuses,=20count=20whole=20plans,=20keep=20b?= =?UTF-8?q?oth=20spend=20facts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings, four bots, three of them converging on the same lines. **A collab agent status reached the page unredacted (security).** This layer IS the redactor — every neighbouring field goes through `context.clean` — and the per-status summary was the one string that did not. A status is provider text, not a closed vocabulary this module controls, so a credential appearing in one would have reached both the parsed JSON and the rendered transcript. **Partially priced retries published a subtotal as the whole cost.** The flag was suppressed whenever `spentUsd > 0`, so a step that spent $0.02 on attempt 1 and an unpriced amount on attempt 2 reported exactly $0.02. The two facts are not exclusive: `costUsd` is then a *lower bound* and `costUnmetered` says so — `run-state.ts`'s `addSpend` keeps them together for the same reason. This is the same failure mode as a fabricated zero, one level up, and I built it in while trying to avoid the other. **And it tested a positive sum rather than a reported price.** An attempt reporting `total_cost_usd: 0` did produce a price; `spentUsd === 0` called that step unpriced. Now keyed on presence. **A 13-item plan whose last item was done rendered `0/13`.** `done` counted the displayed prefix while `total` counted every item — backwards from "the rest are counted, not printed". Counted across the whole plan; only the display is capped. **`reasoningOut` was dead code** (AGENTS.md rule 6). It was populated and never read, and the journal's transcript digest does not record reasoning tokens at all, so the extraction read a field that was never there. Removed. The run page reads the breakdown off Codex's own `turn.completed` frame, where it does exist. `Tests 3455 passed`; the two failures are the standing environment ones (Bun pinned 1.4.0 vs 1.4.2, `hn-monitor` wants a real Claude CLI). Co-Authored-By: Claude Opus 5 (1M context) --- packages/sdk/src/cloud-mirror-step.ts | 37 ++++++++-------- packages/sdk/src/cloud-transcript-codex.ts | 14 ++++++- packages/sdk/tests/cloud-mirror-step.test.ts | 42 +++++++++++++++---- .../sdk/tests/cloud-transcript-codex.test.ts | 33 +++++++++++++++ 4 files changed, 97 insertions(+), 29 deletions(-) diff --git a/packages/sdk/src/cloud-mirror-step.ts b/packages/sdk/src/cloud-mirror-step.ts index ffa9924d4..42b50ac86 100644 --- a/packages/sdk/src/cloud-mirror-step.ts +++ b/packages/sdk/src/cloud-mirror-step.ts @@ -327,17 +327,6 @@ interface AttemptRecord { tokensOut?: number; costUsd?: number; costUnmetered?: boolean; - /** - * Reasoning tokens the provider reported, when it separates them. - * - * Carried as a *breakdown*, never added to `tokensOut`: codex's own - * `blended_total()` is `non_cached_input + output_tokens` and nothing in its - * protocol adds reasoning to output, so `reasoning_output_tokens` is a - * subset of `output_tokens` (codex-rs/protocol/src/protocol.rs). Summing - * them would inflate metered output and, through `maxDollars`, the budget - * ceiling itself. - */ - reasoningOut?: number; } /** @@ -356,7 +345,6 @@ function attemptsByStep(events: readonly JournalEvent[]): Map= 0 ? { costUsd: result['total_cost_usd'] as number } : {}), ...(budget['dollars_unmetered'] === true ? { costUnmetered: true } : {}), - ...(int32(usage?.['reasoning_output_tokens']) === undefined - ? {} : { reasoningOut: int32(usage!['reasoning_output_tokens'])! }), }); byStep.set(event.step_id, entries); } @@ -402,10 +388,23 @@ function finalStep( // Cloud totals these rows for the run's spend — so a retried agent's earlier // charges simply vanished from the run. const spentUsd = attempts.reduce((total, entry) => total + (entry.costUsd ?? 0), 0); - // Unmetered only when no attempt produced a price. A step that was priced on - // one attempt and unpriced on another has a real, if partial, figure — and - // claiming the whole step is unpriced would hide it. - const unmetered = spentUsd === 0 && attempts.some(entry => entry.costUnmetered === true); + const priced = attempts.some(entry => entry.costUsd !== undefined); + /** + * Any unpriced attempt makes the step's cost unknown, even when another + * attempt reported one. + * + * The first cut suppressed the flag whenever `spentUsd > 0`, which published + * a partial subtotal as though it were the whole cost: a step that spent + * $0.02 on attempt 1 and an unpriced amount on attempt 2 reported exactly + * $0.02. The two facts are not exclusive — `costUsd` is then a *lower bound* + * and the flag says so. `run-state.ts`'s `addSpend` keeps them together for + * the same reason. + * + * Presence, not a positive sum: an attempt reporting `total_cost_usd: 0` did + * produce a price, and testing `spentUsd === 0` would have called that step + * unpriced. + */ + const unmetered = attempts.some(entry => entry.costUnmetered === true); const detail = stepDetail(attempts, env); const verification = step.last_attempt?.verification ?? null; // What the step said about itself. A gate's verdict detail is the nearest @@ -439,7 +438,7 @@ function finalStep( ...(model === undefined ? {} : { model }), ...(tokensIn > 0 ? { tokensInput: Math.min(tokensIn, MAX_INT32) } : {}), ...(tokensOut > 0 ? { tokensOutput: Math.min(tokensOut, MAX_INT32) } : {}), - ...(spentUsd > 0 ? { costUsd: spentUsd } : {}), + ...(priced ? { costUsd: spentUsd } : {}), ...(unmetered ? { costUnmetered: true as const } : {}), ...(error === undefined ? {} : { error }), ...(detail.detail === undefined ? {} : { detail: detail.detail }), diff --git a/packages/sdk/src/cloud-transcript-codex.ts b/packages/sdk/src/cloud-transcript-codex.ts index 28b537361..a7d327e94 100644 --- a/packages/sdk/src/cloud-transcript-codex.ts +++ b/packages/sdk/src/cloud-transcript-codex.ts @@ -298,7 +298,13 @@ function collabEntry(item: Record, complete: boolean, context: if (status !== null) byStatus.set(status, (byStatus.get(status) ?? 0) + 1); } } - const summary = [...byStatus.entries()].sort().map(([status, n]) => `${n} ${status}`).join(', '); + // Redacted like every neighbouring field. A status is provider text, not a + // closed vocabulary this module controls, and it was the one string here that + // reached the page without passing the redactor. + const summary = bounded( + [...byStatus.entries()].sort().map(([status, n]) => `${n} ${status}`).join(', '), + context.clean, + ); const status = str(item['status']); return tool('collab_tool_call', bounded(`${name}${receivers > 0 ? ` → ${receivers} thread${receivers === 1 ? '' : 's'}` : ''}`, context.clean), @@ -361,8 +367,12 @@ function itemEntries(item: Record, complete: boolean, context: const text = str(todo?.['text']); return text === null ? [] : [{ text: context.clean(text), done: todo!['completed'] === true }]; }); + // Counted across the whole plan; only the *display* is capped. Counting the + // prefix rendered a 13-item plan whose last item was done as `0/13`, which + // is exactly backwards from "the rest are counted, not printed". + const done = items.filter(entry => isRecord(entry) && entry['completed'] === true).length; return [{ - kind: 'todo', total: items.length, done: listed.filter(entry => entry.done).length, + kind: 'todo', total: items.length, done, items: listed, ...(items.length > listed.length ? { omitted: items.length - listed.length } : {}), ...(complete ? {} : { complete: false }), }]; diff --git a/packages/sdk/tests/cloud-mirror-step.test.ts b/packages/sdk/tests/cloud-mirror-step.test.ts index 5ca2805fa..54d1d9ebc 100644 --- a/packages/sdk/tests/cloud-mirror-step.test.ts +++ b/packages/sdk/tests/cloud-mirror-step.test.ts @@ -246,25 +246,51 @@ describe('unpriced spend', () => { expect(step).not.toHaveProperty('costUsd'); }); - it('prefers a real figure over the unmetered flag when one attempt was priced', () => { + /** + * The two facts are not exclusive. A step priced on one attempt and unpriced + * on another has a cost that is *at least* the known figure — the first cut + * suppressed the flag whenever the sum was positive and published that + * subtotal as the whole cost. + */ + it('keeps the known figure and the unknown marker together', () => { const events = codexStep({ tokens_in: 1, tokens_out: 1, dollars: '0', dollars_unmetered: true }); (events[2]!.payload as Record)['trajectory_tail'] = { transcript: { result: { total_cost_usd: 0.004 } }, }; const [step] = mirrorJournal('01RUN', events, 1_700_000_010_000, {}).finals; - expect(step).toMatchObject({ costUsd: 0.004 }); + // costUsd is a lower bound, and costUnmetered says so. + expect(step).toMatchObject({ costUsd: 0.004, costUnmetered: true }); + }); + + /** + * Presence of a price, not a positive sum: an attempt reporting + * `total_cost_usd: 0` did produce a price, and testing `spentUsd === 0` would + * have reported that step as having none. + */ + it('treats a reported zero as a price, not as absent', () => { + const events = codexStep({ tokens_in: 1, tokens_out: 1, dollars: '0', dollars_unmetered: false }); + (events[2]!.payload as Record)['trajectory_tail'] = { + transcript: { result: { total_cost_usd: 0 } }, + }; + const [step] = mirrorJournal('01RUN', events, 1_700_000_010_000, {}).finals; + + expect(step!.costUsd).toBe(0); expect(step).not.toHaveProperty('costUnmetered'); }); /** - * Codex's own `blended_total()` is `non_cached_input + output_tokens` and - * nothing in its protocol adds reasoning to output, so - * `reasoning_output_tokens` is a SUBSET of `output_tokens`. Carried as a - * breakdown; summing it would inflate metered output and, through - * `maxDollars`, the budget ceiling itself. + * `reasoning_output_tokens` is a SUBSET of `output_tokens` — Codex's own + * `blended_total()` is `non_cached_input + output_tokens` and nothing in its + * protocol adds reasoning to output. Metered output must therefore be the + * journal's `tokens_out` verbatim, never a sum with reasoning added, which + * would inflate it and, through `maxDollars`, the budget ceiling itself. + * + * The breakdown is not carried here: the journal's transcript digest does not + * record it, so there was nothing to read. The run page reads it off Codex's + * own `turn.completed` frame instead. */ - it('carries reasoning tokens as a breakdown, never added to metered output', () => { + it('reports metered output verbatim, never summed with reasoning', () => { const [step] = mirrorJournal('01RUN', codexStep( { tokens_in: 13078, tokens_out: 900, dollars: '0', dollars_unmetered: true }, { input_tokens: 13078, output_tokens: 900, reasoning_output_tokens: 850 }, diff --git a/packages/sdk/tests/cloud-transcript-codex.test.ts b/packages/sdk/tests/cloud-transcript-codex.test.ts index e6b6e10b2..1b986298b 100644 --- a/packages/sdk/tests/cloud-transcript-codex.test.ts +++ b/packages/sdk/tests/cloud-transcript-codex.test.ts @@ -609,6 +609,39 @@ describe('the remaining Codex item types', () => { }); }); + /** + * A status is provider text, not a closed vocabulary this module controls, and + * this layer IS the redactor — every neighbouring field goes through + * `context.clean`. The summary was the one string that did not. + */ + it('redacts a collab agent status like every neighbouring field', () => { + const parsed = parseAgentTranscript(JSON.stringify({ + type: 'item.completed', + item: { id: 'i0', type: 'collab_tool_call', tool: 'spawn_agent', + sender_thread_id: 's', receiver_thread_ids: ['r1'], + agents_states: { a: { status: 'failed: token sk-ant-0123456789abcdefghij' } }, + status: 'failed' }, + }), {}); + + const rendered = JSON.stringify(parsed.entries); + expect(rendered).not.toContain('sk-ant-0123456789abcdefghij'); + expect(rendered).toContain('[redacted]'); + }); + + it('counts completed todos across the whole plan, not the displayed prefix', () => { + // 13 items, only the last one done: the display caps at 12, the count must not. + const items = Array.from({ length: 13 }, (_unused, index) => ({ + text: `task ${index}`, completed: index === 12, + })); + const parsed = parseAgentTranscript(JSON.stringify({ + type: 'item.completed', item: { id: 'i0', type: 'todo_list', items }, + }), {}); + + // Used to render 0/13, which is backwards from "counted, not printed". + expect(parsed.entries.find(entry => entry.kind === 'todo')) + .toMatchObject({ total: 13, done: 1, omitted: 1 }); + }); + it('summarises collab agents by status, never by agent id', () => { const parsed = parseAgentTranscript(JSON.stringify({ type: 'item.completed',