From 868110d2a6739dee58f536eded3e269dd7a71b5e Mon Sep 17 00:00:00 2001 From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:22:13 -0400 Subject: [PATCH 1/3] feat(engine): report real per-turn token usage in the envelope and wrapper events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `--output json` envelope reported `metadata.tokensIn: 0` and `tokensOut: 0` on every invocation regardless of actual usage. The CLI read those keys off the engine turn result, but nothing ever populated them, so the defaults always won. Token data was observable only by consuming the `usage` display events off the ndjson stderr stream, which required `--display ndjson` and left every host to re-derive the totals itself. That aggregation is easy to get wrong: the engine emits one usage event per LLM call plus a trailing rollup carrying zero tokens, so a last-value-wins reader silently reports nothing. The engine now accumulates usage at the display protocol point and reports it directly. That placement sits upstream of the human-facing renderer, so the same numbers appear under `--display text`, `--display ndjson`, and `--quiet`; usage is no longer a side effect of which renderer is attached. Delegated sub-agent calls flow through the same point and are included. Envelope metadata gains three fields: cacheReadTokens int cacheWriteTokens int costUsd str | null `tokensIn` is the charged input total: new input plus cache reads plus cache writes. This matches the field's original definition, "input tokens charged", and avoids the undercount that reporting only new input produces on a cached turn. Hosts wanting the new-input figure derive it by subtracting the two cache fields. `costUsd` is a decimal string rather than a float, because a float loses monetary precision as soon as a host sums turns. An error envelope raised once a turn is running now reports what the turn spent before it failed, rather than a hardcoded zero. The pre-boot argv-validation envelope still reports zero and omits the three new fields: no turn ran, so there is nothing to report. Wrapper SDKs, both in parity: - `ResultEvent` gains `session_id`, `turn_id`, `exit_code`, `usage`, and `stderr_tail`; `ErrorEvent` gains the first four. The parser previously shape-validated the envelope and then discarded everything but `reply`. - A new `Usage` type. `cost_usd` is a `Decimal` in Python but stays a string in TypeScript, since a JS `number` is IEEE-754 and would lose precision. This is a deliberate parity exception and is documented as one. - Synthesized errors that never produced an envelope now carry the session id the wrapper already knows. `turn_id` stays absent, because the engine assigns turn ids and no envelope came back. Envelope values remain authoritative when one exists. - New `stderr_tail_bytes` / `stderrTailBytes` session option governing both terminal events: a positive int caps the tail, `None`/`null` returns the entire buffer, `0` disables it. The default preserves existing behavior. Fixes the stderr tail measuring characters rather than bytes. Both wrappers sliced a string, so `STDERR_TAIL_BYTES` was wrong on any non-ASCII stderr. The tail is now UTF-8 byte-accurate and never splits a codepoint, which means it may return slightly fewer than the requested bytes when trimming to a boundary. PROTOCOL_VERSION moves 0.3.0 to 0.4.0 for the additive envelope fields. Both wrappers, the conformance fixtures, and the documented pins move with it. The two fixtures pinning the `2099-12-future-vN` skew sentinel stay stale deliberately: they exist to prove the engine refuses a foreign version. Coverage: a new `usage` e2e suite, the first to drive a wrapper SDK inside the DTU. Its accuracy case pins the envelope against an independent oracle, summing the provider's own per-call usage out of the session event log and requiring exact equality, so a dropped event, a double count, or an omitted cache bucket fails the build rather than merely looking plausible. Impact: coordinated cross-component. Engine and both wrapper SDKs must be released together; a wrapper pinning 0.3.0 will refuse a 0.4.0 engine. ๐Ÿค– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- CHANGELOG.md | 57 +++ docs/INTEGRATION.md | 9 +- docs/architecture/data-flows.md | 12 +- docs/spec/envelope-and-errors.md | 44 +- docs/spec/install-and-distribution.md | 6 +- docs/spec/wire-protocol.md | 4 +- docs/spec/wrapper-contract.md | 98 ++++- src/amplifier_agent_cli/modes/single_turn.py | 74 +++- src/amplifier_agent_lib/engine.py | 47 ++- .../approval-shim-three-error-codes.yaml | 8 +- .../fixtures/capability_negotiation.yaml | 4 +- .../fixtures/initialize-baseline.yaml | 4 +- .../initialize-with-mcp-config-path.yaml | 4 +- .../conformance/fixtures/l14_synthesis.yaml | 4 +- .../fixtures/resume-with-session-store.yaml | 6 +- .../fixtures/resume_continuity.yaml | 6 +- .../fixtures/subagent_lineage.yaml | 4 +- .../conformance/fixtures/version_skew.yaml | 2 +- src/amplifier_agent_lib/protocol/methods.py | 38 +- .../schemas/TurnSubmitResult.schema.json | 27 ++ src/amplifier_agent_lib/protocol/spec.md | 2 +- .../protocol_points/__init__.py | 2 + .../protocol_points/usage_accumulator.py | 184 ++++++++ tests/e2e/suites/usage/__init__.py | 1 + tests/e2e/suites/usage/conftest.py | 105 +++++ tests/e2e/suites/usage/events_oracle.py | 178 ++++++++ .../e2e/suites/usage/fixtures/usage_driver.py | 281 +++++++++++++ tests/e2e/suites/usage/test_usage_envelope.py | 237 +++++++++++ tests/e2e/suites/usage/test_usage_wrapper.py | 394 ++++++++++++++++++ wrappers/python-py/README.md | 6 +- wrappers/python-py/examples/README.md | 2 +- .../src/amplifier_agent_py/__init__.py | 4 + .../python-py/src/amplifier_agent_py/_api.py | 16 +- .../amplifier_agent_py/run_output_parser.py | 189 ++++++++- .../src/amplifier_agent_py/session.py | 34 +- .../python-py/src/amplifier_agent_py/sync.py | 3 + .../python-py/src/amplifier_agent_py/types.py | 95 ++++- .../src/amplifier_agent_py/version.py | 2 +- wrappers/typescript/dist/argv-builder.d.ts | 2 +- wrappers/typescript/dist/index.d.ts | 30 +- wrappers/typescript/dist/index.js | 15 +- .../typescript/dist/run-output-parser.d.ts | 68 ++- wrappers/typescript/dist/run-output-parser.js | 155 ++++++- wrappers/typescript/dist/session.d.ts | 113 ++++- wrappers/typescript/dist/session.js | 27 +- wrappers/typescript/dist/types.d.ts | 5 + wrappers/typescript/src/argv-builder.ts | 2 +- wrappers/typescript/src/index.ts | 43 +- wrappers/typescript/src/run-output-parser.ts | 193 ++++++++- wrappers/typescript/src/session.ts | 152 ++++++- wrappers/typescript/src/types.ts | 5 + wrappers/typescript/test/argv-builder.test.ts | 6 +- .../test/issue-1-configpath.test.ts | 4 +- .../typescript/test/issue-10-approval.test.ts | 8 +- .../typescript/test/run-output-parser.test.ts | 334 ++++++++++++++- .../test/session-mode-a-shape.test.ts | 63 ++- .../test/session-subprocess.test.ts | 88 +++- wrappers/typescript/test/smoke.test.ts | 2 +- 58 files changed, 3286 insertions(+), 222 deletions(-) create mode 100644 src/amplifier_agent_lib/protocol_points/usage_accumulator.py create mode 100644 tests/e2e/suites/usage/__init__.py create mode 100644 tests/e2e/suites/usage/conftest.py create mode 100644 tests/e2e/suites/usage/events_oracle.py create mode 100644 tests/e2e/suites/usage/fixtures/usage_driver.py create mode 100644 tests/e2e/suites/usage/test_usage_envelope.py create mode 100644 tests/e2e/suites/usage/test_usage_wrapper.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c9e061a..b1613dd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,63 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Real per-turn token and cost usage, on the envelope and on both wrapper SDKs.** + `metadata.tokensIn` / `metadata.tokensOut` were previously hardcoded to `0` on every path; they + now report what the turn actually spent, summed by the engine across every LLM call the turn + made (including calls made by delegated sub-agents). Three new `metadata` fields carry the rest + of the picture: `cacheReadTokens` and `cacheWriteTokens` (both `int`), and `costUsd` (a decimal + STRING, e.g. `"0.00842"`, or `null` when no provider reported a cost -- never a float, since a + float cannot hold a decimal money value exactly and a host summing per-turn costs from floats + accumulates drift it cannot see). `tokensIn` is the CHARGED input total: new input plus both + cache fields; a host wanting the new-only figure derives it as + `tokensIn - cacheReadTokens - cacheWriteTokens`. Usage accounting sits upstream of the CLI's + display renderer, so the same numbers are reported under `--display text`, `--display ndjson`, + and `--quiet` alike. + + The three envelope paths now differ deliberately where they previously didn't: + - the success envelope reports real spend, as above. + - the in-turn error envelope also reports real spend -- whatever the turn burned before it + failed, rather than the previous hardcoded `0`/`0`. A turn that burned tokens and then failed + no longer hides them from whoever is paying for them. + - the pre-boot (argv-validation) envelope is unchanged: still `0`/`0`, and now also omits all + three new fields entirely rather than reporting a zero/null placeholder. No turn ran on that + path, so there is nothing to report. + + Both wrapper SDKs surface the same data on their terminal events. `ResultEvent` gains + `session_id`, `turn_id`, `exit_code`, `usage`, and `stderr_tail`; `ErrorEvent` gains `session_id`, + `turn_id`, `exit_code`, and `usage`. A new `Usage` type carries the five fields above under + wrapper-native naming (`input_tokens` / `inputTokens`, etc.), read straight off the envelope with + no wrapper-side re-summing. `cost_usd` is the one field where the two wrappers deliberately + differ in type: Python parses it into a `Decimal`, TypeScript keeps the exact decimal string, + because JavaScript's `number` is an IEEE-754 binary double and parsing a monetary string into one + would reintroduce the precision loss the wire format exists to avoid. Synthesized errors that + never see an envelope (`spawn_failed`, `engine_hung`, `envelope_missing`, `engine_exit_`) now + carry the `session_id` the wrapper already knows from the caller; `turn_id` stays absent on all + four, since the engine assigns turn ids and none of these paths involves an engine that returned + one. An envelope's own identity fields remain authoritative whenever one exists. + + Both wrappers also gain a `stderr_tail_bytes` option (`stderrTailBytes` in TypeScript) on the + session handle: a positive int caps the terminal event's `stderr_tail` to that many UTF-8 bytes + (never splitting a codepoint), `None`/`null` returns the entire buffer, and `0` disables capture + entirely. It applies uniformly to both `ResultEvent` and `ErrorEvent`, and now also bounds a + `stderrTail` the engine itself supplied inside an error envelope, which previously passed through + uncapped. The default of 4096 bytes preserves the historical behavior exactly. + + `PROTOCOL_VERSION` moves `0.3.0` -> `0.4.0`, purely additive: no existing field changed shape or + meaning, but the minor version moves so a host can gate on the new fields being present. Both + wrappers are pinned to the new value; per the usual rule, a wrapper pinned to `0.3.0` and an + engine on `0.4.0` fail the handshake loudly (`protocol_version_mismatch`) rather than silently + misbehaving, so **upgrading the engine and both wrapper packages together is required.** + +### Fixed + +- **The wrapper's `stderr_tail` cap now measures real UTF-8 bytes, not characters.** It previously + sliced a plain string, so on non-ASCII stderr the cap was wrong by roughly the size of the + encoding (a Japanese reply could overshoot a byte budget several times over). The cap is now + byte-accurate and never splits a codepoint, which means it may return a handful of bytes fewer + than the requested cap when trimming lands mid-character. The exported `STDERR_TAIL_BYTES` + constant keeps its name and its value (4096); it simply counts the right unit now. + - **`amplifier-agent run --prompt-file `,** a second transport for the prompt. The positional `PROMPT` argument remains valid and unchanged; the two are mutually exclusive. File contents are decoded as UTF-8 and delivered verbatim, with no stripping and no newline diff --git a/docs/INTEGRATION.md b/docs/INTEGRATION.md index a6ce9c54..6f7030d3 100644 --- a/docs/INTEGRATION.md +++ b/docs/INTEGRATION.md @@ -116,7 +116,7 @@ Model ids on this surface are namespaced per provider, because a single model li ## Wire protocol -Protocol version **`0.3.0`**, defined in `src/amplifier_agent_lib/protocol/methods.py`. Breaking changes bump it. Wrappers must pass `--protocol-version 0.3.0`; a mismatch returns `protocol_version_mismatch` and exits non-zero rather than silently misbehaving. +Protocol version **`0.4.0`**, defined in `src/amplifier_agent_lib/protocol/methods.py`. Breaking changes bump it. Wrappers must pass `--protocol-version 0.4.0`; a mismatch returns `protocol_version_mismatch` and exits non-zero rather than silently misbehaving. The wrapper passes flags as argv. The engine writes one JSON envelope line to stdout on completion. @@ -141,15 +141,16 @@ The wrapper passes flags as argv. The engine writes one JSON envelope line to st ```json { - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "sessionId": "...", "turnId": "turn-1", "reply": "...", "error": null, "metadata": { - "tokensIn": 0, "tokensOut": 0, "durationMs": 0, + "tokensIn": 0, "tokensOut": 0, "cacheReadTokens": 0, "cacheWriteTokens": 0, + "costUsd": null, "durationMs": 0, "bundleDigest": "...", "engineVersion": "...", - "protocolVersion": "0.3.0", "correlationId": "...", + "protocolVersion": "0.4.0", "correlationId": "...", "activeMode": null } } diff --git a/docs/architecture/data-flows.md b/docs/architecture/data-flows.md index aedbbaac..9d366f5e 100644 --- a/docs/architecture/data-flows.md +++ b/docs/architecture/data-flows.md @@ -45,7 +45,7 @@ __main__.main -> single_turn.run -> _execute_turn -> Engine -> _runtime.handler (`amplifier_agent_lib/protocol_points/defaults_cli.py:46` and `:152`). 7. **Protocol version.** `:771` compares `--protocol-version` against the compiled - `PROTOCOL_VERSION` (`protocol/methods.py:11`, currently `0.3.0`). Strict equality + `PROTOCOL_VERSION` (`protocol/methods.py:11`, currently `0.4.0`). Strict equality unless `host_config.allowProtocolSkew` is set. 8. **Workspace.** `:805` `resolve_workspace(argv, env, cwd)` from @@ -276,10 +276,12 @@ keep the two honest. consumers always see a terminal event. 8. **Failure synthesis.** With no parseable envelope, the wrapper builds an error event - from the exit code and the last 4096 bytes of stderr - (`run-output-parser.ts:23` `STDERR_TAIL_BYTES`). The classification-to-exit-code - table is in `docs/spec/envelope-and-errors.md`; how the wrapper consumes it is in - `docs/spec/wrapper-contract.md`. + from the exit code and a stderr tail capped at `stderrTailBytes` real UTF-8 bytes, + never split mid-codepoint (`run-output-parser.ts:93` `tailStderrBytes`), defaulting + to `STDERR_TAIL_BYTES` (4096) when the session handle does not override it. The + classification-to-exit-code table is in `docs/spec/envelope-and-errors.md`; how the + wrapper consumes it, including the `usage` and identity fields now carried on the + terminal event, is in `docs/spec/wrapper-contract.md`. ## Stream discipline diff --git a/docs/spec/envelope-and-errors.md b/docs/spec/envelope-and-errors.md index e44ed79e..f8b5b700 100644 --- a/docs/spec/envelope-and-errors.md +++ b/docs/spec/envelope-and-errors.md @@ -22,7 +22,7 @@ Under `--output text` stdout is left intact so a human sees the reply as it is p ```json { - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "sessionId": "sess-abc-001", "turnId": "turn-1", "reply": "It is 2:15pm Pacific time.", @@ -30,10 +30,13 @@ Under `--output text` stdout is left intact so a human sees the reply as it is p "metadata": { "tokensIn": 1247, "tokensOut": 89, + "cacheReadTokens": 1024, + "cacheWriteTokens": 0, + "costUsd": "0.00842", "durationMs": 1832, "bundleDigest": "", "engineVersion": "0.12.0", - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "correlationId": "3f2a1b9c-4d5e-4f60-9a7b-1c2d3e4f5061", "activeMode": null } @@ -43,6 +46,18 @@ Under `--output text` stdout is left intact so a human sees the reply as it is p - `sessionId` echoes `--session-id` when supplied, else the session id the engine assigned. - `turnId` is the engine's turn id, defaulting to `"turn-1"`. One turn is submitted per process, so in practice it is always `turn-1`. +- `tokensIn` is the CHARGED input total for the turn: new input tokens plus `cacheReadTokens` plus + `cacheWriteTokens`. The model sees all three as input; the split is a billing distinction only, + so reporting new-input alone would understate a cached turn by orders of magnitude. A caller + that wants the new-only figure derives it as `tokensIn - cacheReadTokens - cacheWriteTokens`. +- `tokensOut`, `cacheReadTokens`, `cacheWriteTokens` are turn-scoped sums across every LLM call the + turn made, including calls made by delegated sub-agents. They are independent of `--display` and + of verbosity (`--quiet`, `-v`, `--debug`): the same numbers are reported no matter which renderer, + if any, was attached. +- `costUsd` is a decimal STRING (e.g. `"0.00842"`), never a JSON number, or `null` when no provider + reported a cost for the turn. A float cannot represent a decimal money value exactly, so a caller + summing per-turn costs from JSON floats accumulates drift it cannot see; `null` is not the same + claim as `"0"` -- it means nobody reported a cost, not that the cost was zero. - `durationMs` is wall-clock time measured around the turn by the CLI, not by the engine. - `activeMode` echoes `--mode` verbatim, or `null`. The mode is non-sticky: omitting `--mode` on a resume returns the field to `null`. @@ -58,7 +73,7 @@ Emitted for failures raised once the turn is running. ```json { - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "sessionId": "sess-abc-001", "turnId": "turn-1", "reply": "", @@ -70,12 +85,15 @@ Emitted for failures raised once the turn is running. "message": "unknown approval action 'review'" }, "metadata": { - "tokensIn": 0, - "tokensOut": 0, + "tokensIn": 342, + "tokensOut": 18, + "cacheReadTokens": 0, + "cacheWriteTokens": 0, + "costUsd": "0.00051", "durationMs": 247, "bundleDigest": "", "engineVersion": "0.12.0", - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "correlationId": "3f2a1b9c-4d5e-4f60-9a7b-1c2d3e4f5061", "activeMode": null } @@ -85,7 +103,10 @@ Emitted for failures raised once the turn is running. Error-path invariants: - `reply` is always `""`. -- `tokensIn` / `tokensOut` are always `0`. +- `tokensIn`, `tokensOut`, `cacheReadTokens`, `cacheWriteTokens`, and `costUsd` report whatever the + turn actually spent before it failed, using the same accounting as the success envelope. A turn + that never reached a provider legitimately reports zero tokens and a `null` cost; a turn that + burned tokens and then failed reports them rather than hiding them behind a placeholder `0`. - `severity` is always `"error"`. The value `"warning"` exists in the wrapper types; the engine never emits it. - `activeMode` is always `null`. @@ -97,7 +118,7 @@ Error-path invariants: ### Pre-boot (argv-validation) envelope -Failures detected before the turn starts emit the same envelope shape with three differences: +Failures detected before the turn starts emit the same envelope shape with four differences: ``` sessionId, turnId always "" @@ -106,6 +127,11 @@ classification "protocol" for every argv and config failure; "engine" for the caller their mode name is wrong would be a lie error.remediation included when the failure has one metadata omits activeMode entirely, rather than setting it to null +metadata tokensIn and tokensOut are still 0, but cacheReadTokens, cacheWriteTokens, + and costUsd are omitted entirely rather than reporting zero/null. No turn + ran on this path, so there is nothing to report for the three new fields -- + omission here is a stronger claim than the in-turn error envelope's zero, + which means a turn ran and spent nothing. ``` This is the path for config errors, workspace-slug rejection, protocol skew, headless approval, @@ -245,7 +271,7 @@ One file per turn: { "argvDigest": "sha256:", "envDigest": "sha256:", - "protocolVersion": "0.3.0", + "protocolVersion": "0.4.0", "exitCode": 0, "correlationId": "3f2a1b9c-4d5e-4f60-9a7b-1c2d3e4f5061", "startedAt": "2026-07-31T15:00:00.000000+00:00", diff --git a/docs/spec/install-and-distribution.md b/docs/spec/install-and-distribution.md index 0b5601b0..c866322b 100644 --- a/docs/spec/install-and-distribution.md +++ b/docs/spec/install-and-distribution.md @@ -203,7 +203,7 @@ when a failure must not break the surrounding install. ## `version --json` ```json -{"version": "0.12.0", "protocolVersion": "0.3.0"} +{"version": "0.12.0", "protocolVersion": "0.4.0"} ``` Exactly two keys. This is the wrapper pre-spawn probe: both SDKs run ` version --json` @@ -246,7 +246,7 @@ Four artifacts version independently: amplifier-agent 0.12.0 engine, the release truth amplifier-agent-ts 0.7.0 TypeScript wrapper SDK amplifier-agent-py 0.3.0 Python wrapper SDK -protocol version 0.3.0 declared by the engine and pinned by each wrapper +protocol version 0.4.0 declared by the engine and pinned by each wrapper ``` Only the protocol version couples them. The engine reports its own version from installed package @@ -255,7 +255,7 @@ metadata. ### The compatibility rule **Strict string equality on the protocol version.** There is NO support window, NO N-1 policy, and -NO compatibility matrix. `0.3.0` and `0.3.1` are as incompatible as `0.3.0` and `9.0.0`. +NO compatibility matrix. `0.4.0` and `0.4.1` are as incompatible as `0.4.0` and `9.0.0`. Three independent enforcement points: diff --git a/docs/spec/wire-protocol.md b/docs/spec/wire-protocol.md index 0b33b5bc..8042b65c 100644 --- a/docs/spec/wire-protocol.md +++ b/docs/spec/wire-protocol.md @@ -36,11 +36,11 @@ side channel rather than to the frame dispatcher. ## Protocol version ``` -0.3.0 +0.4.0 ``` Compared by strict string equality. Semver range matching is not used, and no compatibility window -exists: `0.3.1` against `0.3.0` is a mismatch. +exists: `0.4.1` against `0.4.0` is a mismatch. A mismatch is detected at up to three points, in the order a turn reaches them: diff --git a/docs/spec/wrapper-contract.md b/docs/spec/wrapper-contract.md index 19c54745..9df3740f 100644 --- a/docs/spec/wrapper-contract.md +++ b/docs/spec/wrapper-contract.md @@ -10,7 +10,8 @@ a third wrapper in any language must satisfy. Does not cover the envelope shape beyond the ones a wrapper emits (see `cli.md`). Two reference wrappers ship, TypeScript and Python. They are normatively identical; where they -differ, one of them is wrong. +differ, one of them is wrong -- with exactly one deliberate exception, `Usage.cost_usd`, called out +where it applies below. ## Binary discovery @@ -38,7 +39,7 @@ a mismatch after the engine has completed a full bundle load. The engine's response is exactly two keys: ```json -{"version": "0.12.0", "protocolVersion": "0.3.0"} +{"version": "0.12.0", "protocolVersion": "0.4.0"} ``` Both reference wrappers additionally type an optional `bundleDigest` field on the engine info they @@ -149,6 +150,67 @@ Rule 2 envelope absent, unparseable, or partial -> synthesize from exit code pl The envelope field shape is defined in `envelope-and-errors.md` and is not duplicated here. +### Terminal event fields + +Both the result event (Rule 1, success) and the error event (Rule 1 failure, and every Rule 2 +synthesis) carry `sessionId`, `turnId`, `exitCode`, and `usage`, sourced as follows: + +``` +result event sessionId, turnId always present -- read from the envelope, which + always carries them on this path + exitCode the observed process exit code, reported even though + the envelope already decided the outcome (Rule 1), + so a host can still see a post-flush crash + usage read from the envelope; see below for when it is + absent even here + +error event, sessionId, turnId, usage read from the envelope +Rule 1 + +error event, sessionId present whenever the wrapper itself knows it -- every +Rule 2 failure raised through a session handle was given the + session id at construction time, so it is reported + even though there is no envelope to read it from + turnId absent. The engine assigns turn ids; none came back, + and the wrapper will not invent one + usage absent. Only the envelope reports it; nothing else on + the failure path carries usage + exitCode present when the process actually exited (bad exit + code, missing/unparseable envelope); absent on spawn + failure and on a synthesized hang, where it never did +``` + +`usage` on the failure path is not a duplicate report: nothing else on the failure path carries +usage, so a turn that burned tokens and then failed would otherwise spend them invisibly to the +host. + +`usage`, when present, is a `Usage` value with five fields, read straight off the envelope's +`metadata` block with no wrapper-side arithmetic -- the engine already summed the turn: + +``` +input_tokens / inputTokens mirrors metadata.tokensIn: the CHARGED total (new input + + cache reads + cache writes) +output_tokens / outputTokens mirrors metadata.tokensOut +cache_read_tokens / cacheReadTokens mirrors metadata.cacheReadTokens +cache_write_tokens / cacheWriteTokens mirrors metadata.cacheWriteTokens +cost_usd / costUsd turn cost, or null/None when no provider reported one +``` + +`usage` itself is absent (Python: `None`; TypeScript: omitted) only when the envelope's `metadata` +carried none of the five keys at all -- an engine older than protocol 0.4.0, which never reported +them. That is a different claim from a present `Usage` reading all zeros, which means a turn ran +and genuinely spent nothing. + +**The one deliberate parity exception between the two wrappers is `cost_usd`.** The engine puts +`costUsd` on the wire as a decimal string, never a float, because a float cannot hold a decimal +money value exactly and a host summing per-turn costs from floats accumulates drift it cannot see. +The Python wrapper parses that string into a `Decimal`, since Python has a first-class exact-decimal +type and callers expect one. TypeScript has no decimal type -- `number` is an IEEE-754 binary +double, so parsing the string into one would reintroduce exactly the precision loss the wire format +exists to avoid -- so the TypeScript wrapper hands back the exact string unparsed, letting a host +feed it to whichever decimal library it already uses. This is a language constraint, not drift, and +it is the only field where the two wrappers are allowed to differ in type. + ## stderr Under `--display ndjson`, one JSON object per line. Each parsed object is delivered verbatim; the @@ -160,8 +222,28 @@ Each notification is delivered on two paths: pushed onto the handle's event iter supplied. A host subscribing to both receives every notification twice. Subscribe to one. Everything read from stderr, JSON and non-JSON alike, is also appended to a stderr buffer, so a -crash-time tail still carries wire-event context. On failure the last 4096 characters are attached -as `stderrTail`. The field is omitted entirely when stderr was empty. +crash-time tail still carries wire-event context. The tail is bounded by a `stderrTailBytes` option +(`stderr_tail_bytes` in Python) on the session handle, applied uniformly to BOTH the terminal +success event and the terminal failure event, never just the failure path: + +``` +positive int the last N bytes of stderr, measured in real UTF-8 bytes, never split mid-codepoint +null / None the entire stderr buffer, uncapped +0 capture disabled; the field is omitted entirely +omitted the default, 4096 bytes -- preserves the historical failure-only behavior exactly +``` + +The cap is byte-accurate, not character-accurate: a character count is meaningless the moment +stderr contains non-ASCII, and a naive character slice can overshoot a byte budget by several +times on multibyte text. When a byte cap falls inside a multibyte codepoint, the boundary backs up +to the nearest codepoint start rather than emitting a broken character, so the returned tail may be +up to a few bytes shorter than the cap but always decodes cleanly. + +The same cap also governs a `stderrTail` the ENGINE supplied inside an error envelope's `error` +object: a wrapper re-applies its own bound to whatever the envelope handed it, so `stderrTailBytes: +0` really does mean "give me no stderr" regardless of source. `stderrTail` is present on the +terminal event whenever the resulting tail is non-empty, on both success and failure, and is +omitted when stderr was empty or capture was disabled. ## Exit codes @@ -265,6 +347,14 @@ emission: a host with a 10 second stuck threshold gets 5x margin. A spawn failur surfaces as `{type:"error", code:"spawn_failed", classification:"transport"}`. Whichever of {timeout, exit, spawn error} fires first wins; later events are ignored. +Every one of these synthesized (Rule 2) errors -- `spawn_failed`, `engine_hung`, `envelope_missing`, +and `engine_exit_` -- carries the `sessionId` the handle already knows, because it was supplied +at construction time and no envelope needs to exist for the wrapper to know it. `turnId` stays +absent on all four: the engine assigns turn ids, and none of these paths involves an engine that +got far enough to assign or return one, so inventing one would be a fabrication. This never +overrides an envelope: on the Rule 1 path, the envelope's own `sessionId` is authoritative and the +handle's session id is not consulted. + The wall-clock timeout is opt-in. A default of 10 minutes is exported but never applied automatically; unset, `0`, or negative disables the timer entirely. diff --git a/src/amplifier_agent_cli/modes/single_turn.py b/src/amplifier_agent_cli/modes/single_turn.py index 2de7a101..d2ed1646 100644 --- a/src/amplifier_agent_cli/modes/single_turn.py +++ b/src/amplifier_agent_cli/modes/single_turn.py @@ -36,7 +36,7 @@ from amplifier_agent_lib.persistence import WorkspaceError, resolve_workspace from amplifier_agent_lib.protocol import PROTOCOL_VERSION, server_default_capabilities from amplifier_agent_lib.protocol.errors import AaaError -from amplifier_agent_lib.protocol_points import DisplaySystem +from amplifier_agent_lib.protocol_points import DisplaySystem, UsageAccumulator from amplifier_agent_lib.protocol_points.defaults_cli import ( CliApprovalSystem, CliDisplaySystem, @@ -336,6 +336,47 @@ def _classify(code: str) -> str: return _CLASSIFICATION_BY_CODE.get(code, "engine") +# The five usage keys shared by TurnSubmitResult and envelope metadata, with the +# values reported when nothing was accumulated (no LLM call, or a failure before +# the accumulator existed). Zero tokens and an absent cost, NOT a zero cost: +# "no provider reported a cost" and "the cost was $0.00" are different claims. +_NO_USAGE: dict[str, Any] = { + "tokensIn": 0, + "tokensOut": 0, + "cacheReadTokens": 0, + "cacheWriteTokens": 0, + "costUsd": None, +} + + +def _usage_metadata(source: dict[str, Any] | None) -> dict[str, Any]: + """Pull the five usage fields out of a totals/result dict, coercing types. + + ``source`` is either a ``TurnSubmitResult`` (success path) or a + ``UsageAccumulator.totals()`` dict (error path). Missing or malformed values + fall back to ``_NO_USAGE`` rather than raising: a turn must still be able to + report its outcome when its accounting is broken. + """ + src = source or {} + + def _int(key: str) -> int: + try: + return int(src.get(key, 0) or 0) + except (TypeError, ValueError): + return 0 + + cost = src.get("costUsd") + return { + "tokensIn": _int("tokensIn"), + "tokensOut": _int("tokensOut"), + "cacheReadTokens": _int("cacheReadTokens"), + "cacheWriteTokens": _int("cacheWriteTokens"), + # Decimal STRING or null on the wire. A float loses monetary precision + # the moment a host sums it. + "costUsd": None if cost is None else str(cost), + } + + def _build_error_envelope( *, code: str, @@ -345,11 +386,14 @@ def _build_error_envelope( turn_id: str, duration_ms: int, stderr_tail: str | None = None, + usage: dict[str, Any] | None = None, ) -> dict[str, Any]: classification = _classify(code) metadata: dict[str, Any] = { - "tokensIn": 0, - "tokensOut": 0, + # D6: a turn that burned tokens and THEN failed still reports what it + # spent. Nothing else on the failure path carries usage, so omitting it + # here would make those tokens invisible to the host paying for them. + **_usage_metadata(usage), "durationMs": duration_ms, "bundleDigest": "", "engineVersion": __version__, @@ -396,10 +440,13 @@ def _build_envelope( The mode is non-sticky: hosts read this field to know which mode (if any) is active for the turn, and omitting ``--mode`` on a resume disables a previously-set mode (the field goes back to ``None``). + + The usage fields come straight off the engine's ``TurnSubmitResult``, which + accumulated them from the turn's ``usage`` display events. The CLI does no + arithmetic of its own -- there is exactly one place these numbers are summed. """ metadata: dict[str, Any] = { - "tokensIn": int(result.get("tokensIn", 0) or 0), - "tokensOut": int(result.get("tokensOut", 0) or 0), + **_usage_metadata(result), "durationMs": duration_ms, "bundleDigest": result.get("bundleDigest", ""), "engineVersion": __version__, @@ -806,6 +853,19 @@ def run( stream=sys.stderr, ) + # Wrap the renderer in the usage accumulator HERE rather than letting the + # Engine do it, purely so this function keeps a handle on the totals. The + # error path below has no TurnSubmitResult to read them off -- the turn blew + # up before producing one -- but the tokens were still spent and still have + # to be reported (D6). Engine.__init__ adopts an accumulator it is handed + # instead of wrapping it again, so there is still exactly one of these and + # exactly one set of numbers. + # + # Upstream of the renderer is the whole point: CliDisplaySystem.emit + # early-returns at QUIET, so `--quiet` and `--display text` report the same + # usage as `--display ndjson`. + usage = UsageAccumulator(display) + # (5b) MCP config: the former --mcp-config-path argv flag was removed. # Hosts now supply the path via either (1) host_config["mcp"]["configPath"] # (translated to AMPLIFIER_MCP_CONFIG by _runtime.make_turn_handler) or @@ -842,7 +902,7 @@ def run( fresh=fresh, cwd=cwd, approval=approval, - display=display, + display=usage, provider=provider_name, allow_protocol_skew=bool((host_config or {}).get("allowProtocolSkew", False)), host_config=host_config, @@ -915,6 +975,7 @@ def run( session_id=session_id or "", turn_id="turn-1", duration_ms=duration_ms, + usage=usage.totals(), ) _real_stdout.write(json.dumps(envelope) + "\n") _real_stdout.flush() @@ -940,6 +1001,7 @@ def run( session_id=session_id or "", turn_id="turn-1", duration_ms=duration_ms, + usage=usage.totals(), ) _real_stdout.write(json.dumps(envelope) + "\n") _real_stdout.flush() diff --git a/src/amplifier_agent_lib/engine.py b/src/amplifier_agent_lib/engine.py index cd7a9f99..7cf95000 100644 --- a/src/amplifier_agent_lib/engine.py +++ b/src/amplifier_agent_lib/engine.py @@ -23,7 +23,12 @@ negotiate_capabilities, server_default_capabilities, ) -from amplifier_agent_lib.protocol_points import ApprovalSystem, DisplaySystem, ProtocolPoints +from amplifier_agent_lib.protocol_points import ( + ApprovalSystem, + DisplaySystem, + ProtocolPoints, + UsageAccumulator, +) if TYPE_CHECKING: from amplifier_foundation.bundle._prepared import PreparedBundle @@ -98,7 +103,23 @@ def __init__( protocol_points: ProtocolPoints, ) -> None: self._turn_handler = turn_handler - self._protocol_points = protocol_points + # Wrap the injected display protocol point in a UsageAccumulator so every + # `usage` DisplayEvent the turn produces -- including those from delegated + # sub-agents -- is summed on its way to the renderer. Upstream of the + # renderer on purpose: CliDisplaySystem.emit early-returns at QUIET + # verbosity, so accounting downstream of it would silently report zero + # whenever the host ran quiet. See usage_accumulator.py. + # + # Idempotent: a caller that needs its own handle on the totals (the CLI + # does, to report usage on the error path where no TurnSubmitResult + # exists) may pass a UsageAccumulator in, and we adopt it rather than + # wrapping it twice and double-counting. + display = protocol_points["display"] + self._usage: UsageAccumulator = display if isinstance(display, UsageAccumulator) else UsageAccumulator(display) + self._protocol_points: ProtocolPoints = { + "approval": protocol_points["approval"], + "display": self._usage, + } self._booted: bool = False self._shutdown: bool = False self._session_id: str | None = None @@ -198,7 +219,10 @@ async def submit_turn(self, params: Any) -> TurnSubmitResult: Returns ------- TurnSubmitResult - ``{'reply': , 'turnId': }``. + ``reply`` / ``turnId`` / ``sessionId``, plus the turn's token and + cost totals (``tokensIn``, ``tokensOut``, ``cacheReadTokens``, + ``cacheWriteTokens``, ``costUsd``) as accumulated off the display + event stream. Raises ------ @@ -210,6 +234,11 @@ async def submit_turn(self, params: Any) -> TurnSubmitResult: self._guard_booted() self._guard_not_shutdown() + # Turn-scope the totals. Without this the counts would be cumulative + # across every turn an Engine instance served, which is a different + # number from the one the envelope promises. + self._usage.reset() + ctx = TurnContext( session_id=params["sessionId"], turn_id=params["turnId"], @@ -218,7 +247,17 @@ async def submit_turn(self, params: Any) -> TurnSubmitResult: display=self._protocol_points["display"], ) reply = await self._turn_handler(ctx) - return TurnSubmitResult(reply=reply, turnId=params["turnId"], sessionId=params["sessionId"]) + totals = self._usage.totals() + return TurnSubmitResult( + reply=reply, + turnId=params["turnId"], + sessionId=params["sessionId"], + tokensIn=totals["tokensIn"], + tokensOut=totals["tokensOut"], + cacheReadTokens=totals["cacheReadTokens"], + cacheWriteTokens=totals["cacheWriteTokens"], + costUsd=totals["costUsd"], + ) async def shutdown(self, _params: Any = None) -> AgentShutdownResult: """Shut down the engine. diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/approval-shim-three-error-codes.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/approval-shim-three-error-codes.yaml index 37298678..92ad4201 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/approval-shim-three-error-codes.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/approval-shim-three-error-codes.yaml @@ -11,7 +11,7 @@ description: > surface paths. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final] @@ -24,7 +24,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: @@ -68,7 +68,7 @@ script: method: initialize id: 3 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: @@ -112,7 +112,7 @@ script: method: initialize id: 5 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/capability_negotiation.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/capability_negotiation.yaml index db737f7e..4ab4a0b3 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/capability_negotiation.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/capability_negotiation.yaml @@ -6,7 +6,7 @@ description: > tool/started even if internal hook would. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final] @@ -16,7 +16,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-baseline.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-baseline.yaml index 645c3511..95567812 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-baseline.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-baseline.yaml @@ -12,7 +12,7 @@ description: > wire must be an intentional, reviewed addition, not silent drift. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final] @@ -22,7 +22,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-with-mcp-config-path.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-with-mcp-config-path.yaml index 927bf2eb..96905aec 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-with-mcp-config-path.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/initialize-with-mcp-config-path.yaml @@ -8,7 +8,7 @@ description: > this fixture exercises only the client wire plumbing. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final] @@ -18,7 +18,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/l14_synthesis.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/l14_synthesis.yaml index f59b4830..8602fc5e 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/l14_synthesis.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/l14_synthesis.yaml @@ -7,7 +7,7 @@ description: > other fixture and is therefore not duplicated here. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final] @@ -17,7 +17,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/resume-with-session-store.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/resume-with-session-store.yaml index e3197aad..1c380445 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/resume-with-session-store.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/resume-with-session-store.yaml @@ -8,7 +8,7 @@ description: > continuity is tested in tests/test_resume_continuity.py. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/final, tool/started, tool/completed] @@ -19,7 +19,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: @@ -73,7 +73,7 @@ script: method: initialize id: 3 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/resume_continuity.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/resume_continuity.yaml index 1c9fe60f..22c0cee3 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/resume_continuity.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/resume_continuity.yaml @@ -5,7 +5,7 @@ description: > second turn that can reference first-turn context (design ยง5.3). setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: {display: {events: [result/final]}} script: @@ -14,7 +14,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: {display: {events: [result/final]}} sessionId: sess-resume-1 @@ -42,7 +42,7 @@ script: method: initialize id: 3 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: {display: {events: [result/final]}} sessionId: sess-resume-1 diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/subagent_lineage.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/subagent_lineage.yaml index 11b00dc5..74a58002 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/subagent_lineage.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/subagent_lineage.yaml @@ -6,7 +6,7 @@ description: > field on existing notification payloads. setup: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientCapabilities: display: events: [result/delta, result/final, progress] @@ -16,7 +16,7 @@ script: method: initialize id: 1 params: - protocolVersion: "0.3.0" + protocolVersion: "0.4.0" clientInfo: {name: conformance-harness, version: "0.0.0"} capabilities: display: {events: [result/delta, result/final, progress]} diff --git a/src/amplifier_agent_lib/protocol/conformance/fixtures/version_skew.yaml b/src/amplifier_agent_lib/protocol/conformance/fixtures/version_skew.yaml index e6d1ca1e..bbd35059 100644 --- a/src/amplifier_agent_lib/protocol/conformance/fixtures/version_skew.yaml +++ b/src/amplifier_agent_lib/protocol/conformance/fixtures/version_skew.yaml @@ -27,7 +27,7 @@ script: data: code: protocol_version_mismatch clientVersion: "2099-12-future-vN" - serverVersion: "0.3.0" + serverVersion: "0.4.0" remediation: "Reinstall the matching amplifier-agent and amplifier-agent-client packages, or set allowProtocolSkew: true in the host config file (--config)." assertions: diff --git a/src/amplifier_agent_lib/protocol/methods.py b/src/amplifier_agent_lib/protocol/methods.py index b725ca4c..5d1264f6 100644 --- a/src/amplifier_agent_lib/protocol/methods.py +++ b/src/amplifier_agent_lib/protocol/methods.py @@ -8,9 +8,18 @@ from typing import Any, NotRequired, TypedDict -PROTOCOL_VERSION = "0.3.0" +PROTOCOL_VERSION = "0.4.0" """Wire protocol version. Bump on breaking changes; semver applies. +0.4.0 โ€” Additive: ``TurnSubmitResult`` now reports the turn's real token and + cost usage (``tokensIn``, ``tokensOut``, ``cacheReadTokens``, + ``cacheWriteTokens``, ``costUsd``), accumulated by the engine off the + display event stream. The CLI's ``--output json`` envelope carries the + same five fields in ``metadata`` on both the success and the error path. + ``tokensIn`` is the CHARGED input total (new + cache reads + cache + writes); ``costUsd`` is a decimal STRING or null, never a float. + Purely additive: no existing field changed shape or meaning, but the + minor version moves so hosts can gate on the new fields being present. 0.2.0 โ€” MCP config delivery changed from inline ``mcpServers`` dict to a path string (``mcpConfigPath``) pointing at a JSON file in the format documented by amplifier-module-tool-mcp (top-level ``mcpServers`` key). @@ -109,11 +118,36 @@ class TurnSubmitParams(TypedDict): class TurnSubmitResult(TypedDict): - """Result returned by the ``turn/submit`` JSON-RPC method.""" + """Result returned by the ``turn/submit`` JSON-RPC method. + + The usage fields report what THIS turn actually consumed, summed by the + engine across every LLM call the turn made -- including calls made by + delegated sub-agents. They are turn-scoped, not session-cumulative. + + Zero is a legitimate value: a turn that never reached a provider really did + spend nothing. + """ reply: str | None turnId: str sessionId: str # SC-6 + #: CHARGED input tokens: new input + cache reads + cache writes. The model + #: sees all three as input; the split is a billing distinction only, so + #: reporting new-input alone understates a cached turn by orders of + #: magnitude. Derive new-input as tokensIn - cacheReadTokens - cacheWriteTokens. + tokensIn: int + #: Output tokens generated across the turn. + tokensOut: int + #: The portion of tokensIn that was served from the provider's prompt cache. + cacheReadTokens: int + #: The portion of tokensIn that was written into the provider's prompt cache. + cacheWriteTokens: int + #: Turn cost in USD as a decimal STRING (e.g. "0.0123"), or None when no + #: provider reported a cost. A string, never a float: a float cannot hold a + #: decimal money value exactly, and a host summing per-turn costs from + #: floats accumulates drift it cannot see. None is not zero -- an honest + #: "nobody reported a cost" beats a silently-wrong 0. + costUsd: str | None finalEvent: NotRequired[dict[str, Any]] diff --git a/src/amplifier_agent_lib/protocol/schemas/TurnSubmitResult.schema.json b/src/amplifier_agent_lib/protocol/schemas/TurnSubmitResult.schema.json index c6668b0b..4c2210d3 100644 --- a/src/amplifier_agent_lib/protocol/schemas/TurnSubmitResult.schema.json +++ b/src/amplifier_agent_lib/protocol/schemas/TurnSubmitResult.schema.json @@ -19,14 +19,41 @@ "sessionId": { "type": "string" }, + "tokensIn": { + "type": "integer" + }, + "tokensOut": { + "type": "integer" + }, + "cacheReadTokens": { + "type": "integer" + }, + "cacheWriteTokens": { + "type": "integer" + }, + "costUsd": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "finalEvent": { "type": "object", "additionalProperties": {} } }, "required": [ + "cacheReadTokens", + "cacheWriteTokens", + "costUsd", "reply", "sessionId", + "tokensIn", + "tokensOut", "turnId" ], "additionalProperties": false, diff --git a/src/amplifier_agent_lib/protocol/spec.md b/src/amplifier_agent_lib/protocol/spec.md index 8d5e7f4e..6ac320a9 100644 --- a/src/amplifier_agent_lib/protocol/spec.md +++ b/src/amplifier_agent_lib/protocol/spec.md @@ -6,7 +6,7 @@ # Amplifier Agent โ€” Wire Spec -**Protocol version:** `0.3.0` +**Protocol version:** `0.4.0` **Framing:** JSON-RPC 2.0 over NDJSON over stdio. Stdout carries frames only; stderr is free-form log output. diff --git a/src/amplifier_agent_lib/protocol_points/__init__.py b/src/amplifier_agent_lib/protocol_points/__init__.py index c221e2d9..ca085bca 100644 --- a/src/amplifier_agent_lib/protocol_points/__init__.py +++ b/src/amplifier_agent_lib/protocol_points/__init__.py @@ -18,6 +18,7 @@ CliDisplaySystem, DisplayVerbosity, ) +from amplifier_agent_lib.protocol_points.usage_accumulator import UsageAccumulator __all__ = [ "ApprovalAction", @@ -31,4 +32,5 @@ "DisplaySystem", "DisplayVerbosity", "ProtocolPoints", + "UsageAccumulator", ] diff --git a/src/amplifier_agent_lib/protocol_points/usage_accumulator.py b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py new file mode 100644 index 00000000..e45dd177 --- /dev/null +++ b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py @@ -0,0 +1,184 @@ +"""Turn-scoped token/cost accumulation, as a transparent DisplaySystem decorator. + +Every LLM call the engine makes emits a ``usage`` DisplayEvent through the display +protocol point (``bundle/hook_streaming.py``), including calls made by delegated +sub-agents (those carry an extra ``agentName`` field). That makes the display point +the one place where the whole turn's usage is observable, regardless of which +renderer the host attached or how verbose it is. + +So this is a **decorator around the display protocol point, not a renderer**: + + Engine -> UsageAccumulator -> CliDisplaySystem | JsonDisplaySystem | ... + +Placement is load-bearing. ``CliDisplaySystem.emit`` early-returns at QUIET +verbosity (``defaults_cli.py``), so an accumulator downstream of it would report +zero whenever the user passed ``--quiet``. Sitting upstream means usage accounting +is independent of both the display mode and the verbosity: the numbers are a +property of the turn, not of whoever happened to be watching it. + +The decorator is an **observer, never a filter**. Every event is forwarded to the +wrapped system unchanged, in order, whether or not this class understood it. + +Arithmetic notes (mirrors ``amplifier_agent_http/_event_translator.py``, which was +already forced to correct to exactly this): + +* Input tokens arrive split across three buckets -- ``inputTokens`` (new, full + rate), ``cacheWriteTokens`` (~1.25x) and ``cacheReadTokens`` (~0.1x). The model + sees all three as input; the split is purely a billing distinction. Reporting + only ``inputTokens`` made cached turns look 1000-2000x cheaper than they were. + ``charged_input`` is therefore the sum of all three. +* Usage events are **summed**, never taken-last. The engine emits a trailing + rollup event with ``inputTokens: 0`` / ``outputTokens: 0`` that carries only + ``sessionCostTotal`` (``hook_streaming.on_orchestrator_complete``); a + last-event-wins reader reports zero for the whole turn. +* ``sessionCostTotal`` is deliberately **not** added to ``cost_usd``. It is a + session-wide total collected from the kernel's cost contributions, not a + per-call cost, so adding it to a sum of per-call costs double-counts. +* ``cost`` crosses the wire as a decimal **string** to preserve monetary + precision, and is parsed with ``Decimal``. Never float: summing per-call costs + as binary floats accumulates drift a host cannot see. +""" + +from __future__ import annotations + +from decimal import Decimal, InvalidOperation +from typing import Any + +from amplifier_agent_lib.protocol_points.base import DisplayEvent, DisplaySystem + + +def _to_int(value: Any) -> int: + """Coerce a wire value to a non-negative-ish int, defaulting to 0. + + Deliberately total: a malformed count from a misbehaving provider module must + not abort a turn that has already been paid for. Mirrors the ``_to_int`` in + ``amplifier_agent_http/_event_translator.py``. + """ + if value is None: + return 0 + try: + return int(value) + except (TypeError, ValueError): + return 0 + + +def _to_decimal(value: Any) -> Decimal | None: + """Parse a wire cost into a ``Decimal``, or ``None`` if it is not a number. + + ``str()`` first, always: ``Decimal(0.1)`` captures the binary float's error, + ``Decimal("0.1")`` does not. + """ + if value is None: + return None + try: + return Decimal(str(value)) + except (InvalidOperation, TypeError, ValueError): + return None + + +class UsageAccumulator: + """A DisplaySystem decorator that sums ``usage`` events as they pass through. + + Conforms to the ``DisplaySystem`` protocol, so it can stand in for one + anywhere a display protocol point is accepted. + + Attributes + ---------- + new_input: + Sum of ``inputTokens`` -- input tokens billed at the full rate. + cache_read_tokens: + Sum of ``cacheReadTokens``. + cache_write_tokens: + Sum of ``cacheWriteTokens``. + output_tokens: + Sum of ``outputTokens``. + cost_usd: + Sum of the per-call ``cost`` fields as a ``Decimal``, or ``None`` when no + event carried a cost at all. ``None`` is not zero: an honest "no provider + reported a cost" beats a silently-wrong 0.00. + """ + + def __init__(self, inner: DisplaySystem) -> None: + self._inner = inner + self.new_input: int = 0 + self.cache_read_tokens: int = 0 + self.cache_write_tokens: int = 0 + self.output_tokens: int = 0 + self.cost_usd: Decimal | None = None + + # ------------------------------------------------------------------ + # DisplaySystem protocol + # ------------------------------------------------------------------ + + async def emit(self, event: DisplayEvent) -> None: + """Observe the event, then forward it to the wrapped system unchanged. + + Observation never alters, drops, reorders or delays an event, and never + raises: accounting is a side channel, and a bad counter must not be able + to break the display path or fail a turn. + """ + try: + self._observe(event) + except Exception: # pragma: no cover - defensive; _observe is already total + pass + await self._inner.emit(event) + + # ------------------------------------------------------------------ + # Accumulation + # ------------------------------------------------------------------ + + def _observe(self, event: DisplayEvent) -> None: + """Fold one event into the running totals. Non-usage events are ignored.""" + get = getattr(event, "get", None) + if get is None or get("type") != "usage": + return + + # SUM, never take-last: the trailing sessionCostTotal rollup carries + # zeroes for both token counts and would otherwise erase the turn. + self.new_input += _to_int(get("inputTokens")) + self.cache_read_tokens += _to_int(get("cacheReadTokens")) + self.cache_write_tokens += _to_int(get("cacheWriteTokens")) + self.output_tokens += _to_int(get("outputTokens")) + + # Per-call cost only. `sessionCostTotal` on the rollup event is a + # session-wide figure, not a per-call one -- adding it double-counts. + cost = _to_decimal(get("cost")) + if cost is not None: + self.cost_usd = cost if self.cost_usd is None else self.cost_usd + cost + + # ------------------------------------------------------------------ + # Readout + # ------------------------------------------------------------------ + + @property + def charged_input(self) -> int: + """Total input tokens CHARGED: new + cache reads + cache writes. + + The model saw all three as input. A host that wants the new-only figure + derives it as ``charged_input - cache_read_tokens - cache_write_tokens``. + """ + return self.new_input + self.cache_read_tokens + self.cache_write_tokens + + def totals(self) -> dict[str, Any]: + """Return the totals under their wire names. + + The single place the internal names are mapped onto the wire names shared + by ``TurnSubmitResult`` and the CLI's stdout envelope metadata. ``costUsd`` + is a decimal STRING (or ``None``) so it survives JSON without losing + monetary precision. + """ + return { + "tokensIn": self.charged_input, + "tokensOut": self.output_tokens, + "cacheReadTokens": self.cache_read_tokens, + "cacheWriteTokens": self.cache_write_tokens, + "costUsd": None if self.cost_usd is None else str(self.cost_usd), + } + + def reset(self) -> None: + """Zero every total. Called at the start of each turn to turn-scope them.""" + self.new_input = 0 + self.cache_read_tokens = 0 + self.cache_write_tokens = 0 + self.output_tokens = 0 + self.cost_usd = None diff --git a/tests/e2e/suites/usage/__init__.py b/tests/e2e/suites/usage/__init__.py new file mode 100644 index 00000000..f2d6cbd5 --- /dev/null +++ b/tests/e2e/suites/usage/__init__.py @@ -0,0 +1 @@ +"""E2E suite for real per-turn token usage in the envelope and the Python wrapper.""" diff --git a/tests/e2e/suites/usage/conftest.py b/tests/e2e/suites/usage/conftest.py new file mode 100644 index 00000000..16dc6336 --- /dev/null +++ b/tests/e2e/suites/usage/conftest.py @@ -0,0 +1,105 @@ +"""Fixtures for the usage suite: install the Python wrapper SDK and seed its driver. + +The envelope half of this suite needs nothing beyond the engine, which the DTU profile +already installs. The wrapper half needs ``amplifier_agent_py`` importable INSIDE the +container, built from the same working tree the engine came from -- otherwise the tests +would be checking whatever version of the SDK happens to be published, and a local +change to the wrapper would not be exercised at all. + +That is what the ``#subdirectory=wrappers/python-py`` fragment buys: the DTU's +``url_rewrites`` rule matches ``github.com/microsoft/amplifier-agent`` on a boundary +match, so the URL is redirected to the in-DTU Gitea mirror of the local tree and the +fragment survives to select the wrapper package within it. + +Both fixtures fail LOUDLY rather than skipping. A silent skip here would turn "the SDK +could not be installed" into "the usage tests did not run", which reads as green. +""" + +from __future__ import annotations + +import shlex +from pathlib import Path + +import pytest +from framework import dtu + +FIXTURES = Path(__file__).parent / "fixtures" + +# Host config seeded by DTU provisioning (framework/provisioning/host-config.json): +# anthropic provider, claude-sonnet-5, approval mode yes. Nothing about this suite +# needs a bespoke config, so it reuses that one. +HOST_CONFIG = "/root/e2e/host-config.json" + +# In-DTU location of the wrapper driver. +DRIVER_DEST = "/root/e2e/usage_driver.py" + +# Installed from the SAME mirror the engine came from, so the wrapper under test is the +# local working tree's wrapper. +# +# ``--break-system-packages`` is required because Ubuntu's system interpreter ships a +# PEP 668 ``EXTERNALLY-MANAGED`` marker, which uv honours and refuses ``--system`` +# against. The container is disposable and single-purpose, so relaxing that guard here +# costs nothing. +WRAPPER_SPEC = "git+https://github.com/microsoft/amplifier-agent#subdirectory=wrappers/python-py" +WRAPPER_INSTALL_CMD = f"uv pip install --system --break-system-packages {shlex.quote(WRAPPER_SPEC)}" + + +@pytest.fixture(scope="session") +def wrapper_sdk(dtu_id: str) -> None: + """Install ``amplifier_agent_py`` into the DTU's system interpreter and verify it. + + The verification is a real import rather than a check of the installer's exit code: + a resolver can report success while producing a package that does not import (wrong + subdirectory, missing dependency), and every wrapper test would then fail with an + import error that looks nothing like the feature being absent. + """ + install = dtu.exec_json(dtu_id, ["bash", "-lc", WRAPPER_INSTALL_CMD]) + if install.get("exit_code") != 0: + pytest.fail( + "installing the Python wrapper SDK into the DTU failed.\n" + f"command: {WRAPPER_INSTALL_CMD}\n" + f"exit_code: {install.get('exit_code')}\n" + f"stdout:\n{install.get('stdout', '')}\n" + f"stderr:\n{install.get('stderr', '')}" + ) + + verify = dtu.exec_json(dtu_id, ["python3", "-c", "import amplifier_agent_py"]) + if verify.get("exit_code") != 0: + pytest.fail( + "the wrapper SDK installed but does not import in the DTU.\n" + f"exit_code: {verify.get('exit_code')}\n" + f"stdout:\n{verify.get('stdout', '')}\n" + f"stderr:\n{verify.get('stderr', '')}" + ) + + +@pytest.fixture(scope="session") +def engine_bin(dtu_id: str) -> str: + """Resolve the engine binary path inside the DTU. + + ``uv tool install`` puts ``amplifier-agent`` on PATH, so ``shutil.which`` in the + wrapper would find it unaided. Resolving it here anyway and pinning + ``AMPLIFIER_AGENT_BIN`` in the driver removes binary discovery as a possible + explanation for a failed usage assertion. + """ + result = dtu.exec_json(dtu_id, ["bash", "-lc", "command -v amplifier-agent"]) + path = result.get("stdout", "").strip() + if result.get("exit_code") != 0 or not path: + pytest.fail( + "amplifier-agent is not on PATH inside the DTU; the wrapper has no engine to spawn.\n" + f"exit_code: {result.get('exit_code')}\nstderr:\n{result.get('stderr', '')}" + ) + return path + + +@pytest.fixture(scope="session") +def usage_driver(dtu_id: str, wrapper_sdk: None) -> str: + """Push the wrapper driver into the DTU; return its in-DTU path. + + ``wrapper_sdk`` is requested for ordering only: the driver is useless until the SDK + it imports is installed, and depending on it here means a failed install is reported + once, by the fixture that owns it. + """ + del wrapper_sdk + dtu.push_file(dtu_id, str(FIXTURES / "usage_driver.py"), DRIVER_DEST) + return DRIVER_DEST diff --git a/tests/e2e/suites/usage/events_oracle.py b/tests/e2e/suites/usage/events_oracle.py new file mode 100644 index 00000000..6dba515e --- /dev/null +++ b/tests/e2e/suites/usage/events_oracle.py @@ -0,0 +1,178 @@ +"""Independent ground truth for a turn's token usage, read from ``events.jsonl``. + +The envelope's ``metadata`` block is the thing under test, so it cannot also be the +thing that proves itself. This module supplies the second, independent source: the +provider's own per-call usage as recorded by hook-context-intelligence in the +session's ``context-intelligence/events.jsonl``. + +Record shape (one JSON object per line, same as the raw-capture suite reads): + + {"event": "llm:response", "data": {"turn_id": "...", "usage": {...}}} + +and the kernel's ``usage`` sub-dict carries ``input_tokens``, ``output_tokens``, +``cache_read_tokens``, ``cache_write_tokens`` and (when the provider reports one) +``cost_usd``. Note ``input_tokens`` there is the NEW input only -- cache reads and +cache writes are counted separately, which is why the charged total is a sum of the +three rather than ``input_tokens`` alone. + +The file is summed INSIDE the container by a small ``python3 -c`` program and only +the totals cross back to the host. ``events.jsonl`` lines can be very large (a single +line carries a whole LLM request or response when raw capture is on), so ``cat``-ing +the file back would be both slow and a good way to blow up the test process. +""" + +from __future__ import annotations + +import json +import shlex +from dataclasses import dataclass + +from framework import dtu + +# Root that buckets session state by workspace (persistence.workspaces_root()). +# Same constant the raw_capture suite uses; duplicated rather than imported so this +# suite stays a self-contained brick. +WORKSPACES_ROOT = "/root/.amplifier-agent/state/workspaces" + +# Path to the event log, relative to a session directory. +EVENTS_RELPATH = "context-intelligence/events.jsonl" + +# Summing program, run inside the DTU. Reads the log line by line and prints ONE +# small JSON object. Keeping it a separate argv element (python3 -c PROGRAM PATH) +# means no shell quoting is involved in either the program or the path. +_SUM_PROGRAM = r""" +import json, sys + +path = sys.argv[1] +keys = ("input_tokens", "output_tokens", "cache_read_tokens", "cache_write_tokens") +totals = dict.fromkeys(keys, 0) +responses = 0 +responses_without_usage = 0 +turn_ids = [] + +with open(path, encoding="utf-8") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + obj = json.loads(line) + except ValueError: + continue + if not isinstance(obj, dict) or obj.get("event") != "llm:response": + continue + responses += 1 + data = obj.get("data") + if not isinstance(data, dict): + responses_without_usage += 1 + continue + turn_id = data.get("turn_id") + if isinstance(turn_id, str) and turn_id and turn_id not in turn_ids: + turn_ids.append(turn_id) + usage = data.get("usage") + if not isinstance(usage, dict): + responses_without_usage += 1 + continue + for key in keys: + value = usage.get(key) + try: + totals[key] += int(value or 0) + except (TypeError, ValueError): + pass + +json.dump( + { + "totals": totals, + "responses": responses, + "responses_without_usage": responses_without_usage, + "turn_ids": turn_ids, + }, + sys.stdout, +) +""" + + +@dataclass(frozen=True) +class ProviderUsage: + """Per-turn provider usage summed from ``llm:response`` records. + + ``input_tokens`` is the NEW input the provider billed as fresh; ``charged_input`` + adds the cached halves, which is what the envelope's ``tokensIn`` is specified to + report. + """ + + input_tokens: int + output_tokens: int + cache_read_tokens: int + cache_write_tokens: int + responses: int + responses_without_usage: int + turn_ids: tuple[str, ...] + + @property + def charged_input(self) -> int: + """CHARGED input total: new input + cache reads + cache writes.""" + return self.input_tokens + self.cache_read_tokens + self.cache_write_tokens + + +def resolve_session_dir(dtu_id: str, session_id: str, *, root: str = WORKSPACES_ROOT) -> str: + """Return the absolute in-DTU session directory for ``session_id``. + + The workspace slug is picked at runtime, so the directory is found rather than + constructed. A missing or ambiguous match is a hard failure: with no record there + is nothing to compare the envelope against, and a silent fallback would let a test + pass for the wrong reason. + """ + cmd = f"find {shlex.quote(root)} -maxdepth 3 -type d -name {shlex.quote(session_id)} 2>/dev/null" + result = dtu.exec_json(dtu_id, ["bash", "-lc", cmd]) + matches = [line.strip() for line in result.get("stdout", "").splitlines() if line.strip()] + + if not matches: + listing = dtu.exec_json(dtu_id, ["bash", "-lc", f"ls -1 {shlex.quote(root)} 2>&1"]) + raise AssertionError( + f"no session directory named {session_id!r} under {root}.\n" + f"The turn did not persist a session record, so there is no independent\n" + f"usage record to check the envelope against.\n" + f"workspaces present:\n{listing.get('stdout', '')}" + ) + if len(matches) > 1: + raise AssertionError( + f"ambiguous session id {session_id!r}; matched {len(matches)} dirs:\n" + "\n".join(matches) + ) + return matches[0] + + +def read_provider_usage(dtu_id: str, session_id: str) -> ProviderUsage: + """Sum the provider's own usage for ``session_id`` from inside the DTU.""" + session_dir = resolve_session_dir(dtu_id, session_id) + events_path = f"{session_dir}/{EVENTS_RELPATH}" + + result = dtu.exec_json(dtu_id, ["python3", "-c", _SUM_PROGRAM, events_path]) + if result.get("exit_code") != 0: + raise AssertionError( + f"could not sum {events_path} (exit {result.get('exit_code')}).\nstderr:\n{result.get('stderr', '')}" + ) + + stdout = result.get("stdout", "").strip() + try: + payload = json.loads(stdout) + except json.JSONDecodeError as exc: + raise AssertionError(f"usage oracle produced non-JSON output for {events_path}:\n{stdout}") from exc + + if payload["responses"] == 0: + raise AssertionError( + f"{events_path} recorded no 'llm:response' events for session {session_id!r}.\n" + "Without at least one provider-reported usage record there is nothing to\n" + "check the envelope against, so this test cannot prove anything." + ) + + totals = payload["totals"] + return ProviderUsage( + input_tokens=int(totals["input_tokens"]), + output_tokens=int(totals["output_tokens"]), + cache_read_tokens=int(totals["cache_read_tokens"]), + cache_write_tokens=int(totals["cache_write_tokens"]), + responses=int(payload["responses"]), + responses_without_usage=int(payload["responses_without_usage"]), + turn_ids=tuple(payload["turn_ids"]), + ) diff --git a/tests/e2e/suites/usage/fixtures/usage_driver.py b/tests/e2e/suites/usage/fixtures/usage_driver.py new file mode 100644 index 00000000..2333503b --- /dev/null +++ b/tests/e2e/suites/usage/fixtures/usage_driver.py @@ -0,0 +1,281 @@ +#!/usr/bin/env python3 +"""In-DTU driver: run ONE turn through ``amplifier_agent_py`` and describe the result. + +Runs inside the DTU container, not on the host. Prints exactly ONE JSON line to +stdout describing the terminal ``DisplayEvent`` the wrapper produced, so the host-side +test can assert on the wrapper's PUBLIC surface without importing the SDK itself. + +Contract with the caller: + +* stdout -- exactly one JSON object, on the last line. Nothing else. +* stderr -- free-form diagnostics; the caller ignores it except when reporting. +* exit -- always 0 when a report was produced. A non-zero exit means the driver + itself could not run, which is an infrastructure failure, not a + feature failure. + +The driver NEVER asserts. It reports what it found (including which contract +attributes were absent) and lets the test decide. That keeps a missing attribute a +readable test failure rather than an opaque ``AttributeError`` traceback. + +Usage: + python3 usage_driver.py --session-id SID --prompt TEXT [--config PATH] + [--display-mode {text,ndjson}] [--engine-bin PATH] + [--stderr-tail-bytes {N|none}] + +``--stderr-tail-bytes`` is deliberately tri-state: omitted means the kwarg is NOT +passed to ``spawn_agent_sync`` at all (exercising the default), ``none`` passes the +Python value ``None`` (full buffer), and an integer passes that integer. +""" + +from __future__ import annotations + +import argparse +import json +import os +import shutil +import sys +import traceback +from typing import Any + +# Environment the engine subprocess is allowed to inherit. +# +# The SDK's DEFAULT_ALLOWLIST is deliberately minimal (PATH/HOME/USER/LANG/TERM/TMPDIR) +# and carries no provider credential, so a turn spawned with the default allowlist has +# no API key and fails for a reason that has nothing to do with token usage. The extra +# names below are what the DTU itself puts in the environment: the provider credential, +# and the TLS/proxy variables the container's interception proxy needs. +ENGINE_ENV_ALLOWLIST = [ + "PATH", + "HOME", + "USER", + "LANG", + "TERM", + "TMPDIR", + "ANTHROPIC_API_KEY", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "REQUESTS_CA_BUNDLE", + "CURL_CA_BUNDLE", + "NODE_EXTRA_CA_CERTS", + "UV_NATIVE_TLS", + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "http_proxy", + "https_proxy", + "no_proxy", +] + +# Attributes ResultEvent / ErrorEvent are specified to carry once per-turn usage lands. +CONTRACT_ATTRS = ("session_id", "turn_id", "exit_code", "usage", "stderr_tail") + +# Attributes the Usage value object is specified to carry. +USAGE_ATTRS = ("input_tokens", "output_tokens", "cache_read_tokens", "cache_write_tokens", "cost_usd") + +# Unicode REPLACEMENT CHARACTER. Its presence in a byte-bounded stderr tail means the +# slice landed mid-codepoint and was decoded with errors="replace". +REPLACEMENT_CHAR = "\ufffd" + +_ABSENT = object() + + +def _emit(payload: dict[str, Any]) -> None: + """Write the single-line JSON report to stdout.""" + sys.stdout.write(json.dumps(payload, sort_keys=True) + "\n") + sys.stdout.flush() + + +def _note(message: str) -> None: + """Driver diagnostics go to stderr so stdout stays a single JSON line.""" + sys.stderr.write(f"[usage_driver] {message}\n") + + +def _describe_usage(usage: Any) -> dict[str, Any]: + """Describe a Usage value object without assuming any attribute exists.""" + missing: list[str] = [] + values: dict[str, Any] = {} + + for name in USAGE_ATTRS: + value = getattr(usage, name, _ABSENT) + if value is _ABSENT: + missing.append(name) + continue + if name == "cost_usd": + # Decimal is not JSON-serializable, and the test needs to know the type it + # arrived as, so report both the rendering and the type name. + values["cost_usd"] = None if value is None else str(value) + values["cost_usd_type"] = type(value).__name__ + continue + values[name] = value if isinstance(value, int) else str(value) + + return {"missing_attrs": missing, "values": values, "repr": repr(usage)} + + +def _describe_event(event: Any) -> dict[str, Any]: + """Describe the terminal DisplayEvent, recording which contract attrs are absent.""" + missing: list[str] = [] + report: dict[str, Any] = { + "event_type": getattr(event, "type", None), + "text": getattr(event, "text", None), + "code": getattr(event, "code", None), + "message": getattr(event, "message", None), + } + + for name in CONTRACT_ATTRS: + if getattr(event, name, _ABSENT) is _ABSENT: + missing.append(name) + + report["missing_attrs"] = missing + report["session_id"] = getattr(event, "session_id", None) + report["turn_id"] = getattr(event, "turn_id", None) + report["exit_code"] = getattr(event, "exit_code", None) + + usage = getattr(event, "usage", _ABSENT) + if usage is _ABSENT or usage is None: + report["usage"] = None + report["usage_missing_attrs"] = [] + else: + described = _describe_usage(usage) + report["usage"] = described["values"] + report["usage_missing_attrs"] = described["missing_attrs"] + report["usage_repr"] = described["repr"] + + tail = getattr(event, "stderr_tail", _ABSENT) + if tail is _ABSENT or tail is None: + report["stderr_tail"] = None + report["stderr_tail_utf8_len"] = None + report["stderr_tail_char_len"] = None + report["stderr_tail_has_replacement_char"] = False + report["stderr_tail_is_ascii"] = True + else: + text = str(tail) + report["stderr_tail"] = text + report["stderr_tail_utf8_len"] = len(text.encode("utf-8")) + report["stderr_tail_char_len"] = len(text) + report["stderr_tail_has_replacement_char"] = REPLACEMENT_CHAR in text + report["stderr_tail_is_ascii"] = text.isascii() + + return report + + +def _parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--session-id", required=True) + parser.add_argument("--prompt", required=True) + parser.add_argument("--config", default=None) + parser.add_argument("--display-mode", choices=("text", "ndjson"), default=None) + parser.add_argument("--engine-bin", default=None) + parser.add_argument( + "--stderr-tail-bytes", + default=None, + help="Integer byte cap, or the literal 'none' for the full buffer. Omit to exercise the default.", + ) + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = _parse_args(argv) + + # Pin the engine binary explicitly when the caller resolved one, so binary + # discovery can never be the reason a usage assertion fails. + if args.engine_bin: + os.environ["AMPLIFIER_AGENT_BIN"] = args.engine_bin + elif shutil.which("amplifier-agent"): + os.environ.setdefault("AMPLIFIER_AGENT_BIN", str(shutil.which("amplifier-agent"))) + + try: + from amplifier_agent_py import spawn_agent_sync + except Exception as exc: # pragma: no cover - reported, not raised + _emit( + { + "ok": False, + "error_kind": "sdk_import_failed", + "error": f"{type(exc).__name__}: {exc}", + "traceback": traceback.format_exc(), + } + ) + return 0 + + kwargs: dict[str, Any] = { + "session_id": args.session_id, + "approval": {"mode": "yes"}, + "env": {"allowlist": ENGINE_ENV_ALLOWLIST}, + } + if args.config: + kwargs["config_path"] = args.config + if args.display_mode: + kwargs["display_mode"] = args.display_mode + if args.stderr_tail_bytes is not None: + raw = args.stderr_tail_bytes.strip().lower() + kwargs["stderr_tail_bytes"] = None if raw == "none" else int(raw) + + requested_tail = "" if args.stderr_tail_bytes is None else kwargs["stderr_tail_bytes"] + _note(f"spawning session={args.session_id} display={args.display_mode} stderr_tail_bytes={requested_tail}") + + try: + handle = spawn_agent_sync(**kwargs) + except TypeError as exc: + # The option does not exist on this build of the SDK. Report it as data so the + # host-side test can name the missing option instead of showing a traceback. + _emit( + { + "ok": False, + "error_kind": "spawn_kwarg_rejected", + "error": f"{type(exc).__name__}: {exc}", + "rejected_kwargs": sorted(kwargs), + "traceback": traceback.format_exc(), + } + ) + return 0 + except Exception as exc: # pragma: no cover - reported, not raised + _emit( + { + "ok": False, + "error_kind": "spawn_failed", + "error": f"{type(exc).__name__}: {exc}", + "traceback": traceback.format_exc(), + } + ) + return 0 + + terminal: Any = None + try: + with handle: + for event in handle.submit(args.prompt): + if getattr(event, "type", "") in ("result", "error"): + terminal = event + break + except Exception as exc: # pragma: no cover - reported, not raised + _emit( + { + "ok": False, + "error_kind": "submit_failed", + "error": f"{type(exc).__name__}: {exc}", + "traceback": traceback.format_exc(), + } + ) + return 0 + + if terminal is None: + _emit( + { + "ok": False, + "error_kind": "no_terminal_event", + "error": "the event stream ended without a 'result' or 'error' event", + } + ) + return 0 + + report = _describe_event(terminal) + report["ok"] = True + report["error_kind"] = None + report["error"] = None + report["requested_session_id"] = args.session_id + report["requested_stderr_tail_bytes"] = None if args.stderr_tail_bytes is None else kwargs["stderr_tail_bytes"] + report["stderr_tail_bytes_requested"] = args.stderr_tail_bytes is not None + _emit(report) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/e2e/suites/usage/test_usage_envelope.py b/tests/e2e/suites/usage/test_usage_envelope.py new file mode 100644 index 00000000..cca29367 --- /dev/null +++ b/tests/e2e/suites/usage/test_usage_envelope.py @@ -0,0 +1,237 @@ +"""DTU-backed tests for real per-turn token usage in the ``--output json`` envelope. + +Contract under test: after a turn completes, the stdout envelope's ``metadata`` block +reports what the turn actually cost. + + tokensIn int CHARGED input total = new input + cache reads + cache writes + tokensOut int output tokens + cacheReadTokens int the cached half of tokensIn that was read back + cacheWriteTokens int the cached half of tokensIn that was written + costUsd str | None decimal STRING, never a float; None when no provider + reported a cost + +Today ``tokensIn`` and ``tokensOut`` are hardcoded to ``0`` +(``src/amplifier_agent_cli/modes/single_turn.py``) and the three new fields do not +exist at all, so every case here is ``xfail(strict=True)``. Strict means the moment +usage accounting lands these turn XPASS and the markers must come off -- see +docs/E2E_TESTING.md, "Tests for features that do not exist yet". + +The assertions are on the envelope's public shape only. Nothing here reads a log line, +an internal counter, or a private attribute, so a refactor that keeps the envelope +honest keeps these green. + +The last case is the accuracy oracle and the one that actually pins the numbers to +reality. Everything above it would still pass if the engine reported plausible-looking +but wrong totals; that case sums the provider's own per-call usage out of the session's +``context-intelligence/events.jsonl`` and requires the envelope to match it exactly. +""" + +from __future__ import annotations + +import json +import uuid +from typing import Any + +import pytest +from framework import dtu + +from suites.usage.events_oracle import read_provider_usage + +pytestmark = pytest.mark.dtu + +# Host config seeded by DTU provisioning: anthropic / claude-sonnet-5 / approval yes. +CFG = "/root/e2e/host-config.json" + +# Short, tool-free, and delegation-free on purpose. A turn that spawns a sub-agent +# writes its LLM calls into a DIFFERENT session directory, which would put the +# accuracy oracle and the envelope on different sides of a boundary neither of them +# describes. One prompt, one session, one set of numbers. +PROMPT = "Reply with the single word: pong" + + +def _session_id(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _run_turn(dtu_id: str, session_id: str, extra_args: list[str]) -> dict[str, Any]: + """Run one ``--output json`` turn inside the DTU and return the parsed envelope.""" + argv = [ + "amplifier-agent", + "run", + "-y", + "--config", + CFG, + "--output", + "json", + "--session-id", + session_id, + "--fresh", + *extra_args, + PROMPT, + ] + result = dtu.exec_json(dtu_id, argv) + + assert result.get("exit_code") == 0, ( + f"turn failed (exit {result.get('exit_code')}).\n" + f"argv: {argv}\n" + f"stdout:\n{result.get('stdout', '')}\nstderr:\n{result.get('stderr', '')}" + ) + + stdout = result.get("stdout", "").strip() + try: + envelope = json.loads(stdout) + except json.JSONDecodeError as exc: + raise AssertionError( + "stdout was not a parseable envelope under --output json.\n" + "Under --output json the envelope is the ONLY thing on stdout.\n" + f"argv: {argv}\nstdout:\n{stdout}" + ) from exc + + assert envelope.get("error") is None, f"turn returned an error envelope: {envelope.get('error')}" + return envelope + + +def _metadata(envelope: dict[str, Any]) -> dict[str, Any]: + metadata = envelope.get("metadata") + assert isinstance(metadata, dict), f"envelope has no metadata object: {envelope!r}" + return metadata + + +# Three ways of asking for the same envelope. `--display` and `--quiet` govern STDERR; +# `--output json` governs STDOUT. They are independent knobs, so usage accounting must +# not be a side effect of whichever human-facing renderer happened to be attached -- +# a host that runs quiet gets the same numbers as one that streams ndjson. +_DISPLAY_VARIANTS = [ + pytest.param([], id="usage-envelope-tokens-nonzero"), + pytest.param(["--display", "text"], id="usage-envelope-text-display"), + pytest.param(["--quiet"], id="usage-envelope-quiet"), +] + + +@pytest.mark.parametrize("extra_args", _DISPLAY_VARIANTS) +def test_usage_envelope_tokens_nonzero(dtu_id: str, extra_args: list[str]) -> None: + """A completed turn reports non-zero input and output tokens.""" + metadata = _metadata(_run_turn(dtu_id, _session_id("usage-env"), extra_args)) + + tokens_in = metadata.get("tokensIn") + tokens_out = metadata.get("tokensOut") + + assert isinstance(tokens_in, int) and not isinstance(tokens_in, bool), ( + f"metadata.tokensIn must be an int, got {tokens_in!r} ({type(tokens_in).__name__})" + ) + assert isinstance(tokens_out, int) and not isinstance(tokens_out, bool), ( + f"metadata.tokensOut must be an int, got {tokens_out!r} ({type(tokens_out).__name__})" + ) + assert tokens_in > 0, ( + f"metadata.tokensIn is {tokens_in}. A turn that reached the provider always " + "consumed input tokens; 0 means the envelope is reporting a placeholder." + ) + assert tokens_out > 0, ( + f"metadata.tokensOut is {tokens_out}. The turn produced a reply " + f"({metadata.get('durationMs')}ms), so the provider billed output tokens." + ) + + +def test_usage_envelope_breakdown_fields(dtu_id: str) -> None: + """The cache breakdown is present and typed; cost is a decimal STRING or null. + + ``costUsd`` being a string is the load-bearing half. A float cannot represent a + decimal money value exactly, so a host that sums per-turn costs from JSON floats + accumulates drift it cannot see. The type is therefore part of the contract, and + the assertion rejects ``int`` too -- a whole-dollar cost serialized as ``0`` is + the same bug wearing a different hat. + """ + metadata = _metadata(_run_turn(dtu_id, _session_id("usage-breakdown"), [])) + + for field in ("cacheReadTokens", "cacheWriteTokens"): + assert field in metadata, f"metadata.{field} is absent. Keys present: {sorted(metadata)}" + value = metadata[field] + assert isinstance(value, int) and not isinstance(value, bool), ( + f"metadata.{field} must be an int, got {value!r} ({type(value).__name__})" + ) + assert value >= 0, f"metadata.{field} must not be negative, got {value}" + + assert "costUsd" in metadata, f"metadata.costUsd is absent. Keys present: {sorted(metadata)}" + cost = metadata["costUsd"] + assert cost is None or isinstance(cost, str), ( + f"metadata.costUsd must be a decimal string or null, got {cost!r} ({type(cost).__name__}). " + "A float or int loses monetary precision the moment a host sums it." + ) + + +def test_usage_envelope_breakdown_consistent(dtu_id: str) -> None: + """``tokensIn`` is the charged total, so the cached halves cannot exceed it. + + ``tokensIn`` is specified as new input + cache reads + cache writes. A host that + wants the "new" figure derives it by subtracting the two cache fields, so if the + parts ever exceed the whole that subtraction goes negative and every downstream + cost calculation is wrong in a way no type check would catch. + """ + metadata = _metadata(_run_turn(dtu_id, _session_id("usage-consistent"), [])) + + tokens_in = metadata.get("tokensIn") + cache_read = metadata.get("cacheReadTokens") + cache_write = metadata.get("cacheWriteTokens") + + assert isinstance(tokens_in, int), f"metadata.tokensIn must be an int, got {tokens_in!r}" + assert isinstance(cache_read, int), f"metadata.cacheReadTokens must be an int, got {cache_read!r}" + assert isinstance(cache_write, int), f"metadata.cacheWriteTokens must be an int, got {cache_write!r}" + + assert tokens_in >= cache_read + cache_write, ( + f"metadata.tokensIn ({tokens_in}) is smaller than cacheReadTokens + cacheWriteTokens " + f"({cache_read} + {cache_write} = {cache_read + cache_write}). tokensIn is the CHARGED " + "total, so the derived new-input figure (tokensIn - cacheRead - cacheWrite) would be negative." + ) + + +def test_usage_envelope_accuracy_vs_raw_events(dtu_id: str) -> None: + """THE ACCURACY ORACLE: the envelope must equal the provider's own reported usage. + + Every other case in this module would still pass if the engine reported numbers + that were merely plausible. This one runs a turn under a known ``--session-id``, + then sums the provider's per-call usage out of that session's + ``context-intelligence/events.jsonl`` -- a record the engine writes for a different + reason, through a different code path, from the same provider responses -- and + requires exact equality. + + Exact, not approximate. Two independent readings of the same provider response have + no legitimate reason to differ by even one token, and a tolerance would hide the + exact class of bug this exists to catch: a total that drops one call, double-counts + another, or silently omits the cached halves. + """ + session_id = _session_id("usage-oracle") + metadata = _metadata(_run_turn(dtu_id, session_id, [])) + provider = read_provider_usage(dtu_id, session_id) + + assert provider.responses_without_usage == 0, ( + f"{provider.responses_without_usage} of {provider.responses} llm:response records " + "carry no usage sub-dict, so the oracle's total is incomplete and cannot be " + "compared against the envelope." + ) + + context = ( + f"session={session_id} " + f"llm_responses={provider.responses} turn_ids={list(provider.turn_ids)}\n" + f"provider totals: new_input={provider.input_tokens} output={provider.output_tokens} " + f"cache_read={provider.cache_read_tokens} cache_write={provider.cache_write_tokens} " + f"charged_input={provider.charged_input}\n" + f"envelope metadata: {json.dumps({k: metadata.get(k) for k in sorted(metadata)}, default=str)}" + ) + + assert metadata.get("tokensOut") == provider.output_tokens, ( + f"metadata.tokensOut ({metadata.get('tokensOut')}) != provider-reported output " + f"tokens ({provider.output_tokens}).\n{context}" + ) + assert metadata.get("cacheReadTokens") == provider.cache_read_tokens, ( + f"metadata.cacheReadTokens ({metadata.get('cacheReadTokens')}) != provider-reported " + f"cache reads ({provider.cache_read_tokens}).\n{context}" + ) + assert metadata.get("cacheWriteTokens") == provider.cache_write_tokens, ( + f"metadata.cacheWriteTokens ({metadata.get('cacheWriteTokens')}) != provider-reported " + f"cache writes ({provider.cache_write_tokens}).\n{context}" + ) + assert metadata.get("tokensIn") == provider.charged_input, ( + f"metadata.tokensIn ({metadata.get('tokensIn')}) != the provider's CHARGED input total " + f"({provider.charged_input} = new {provider.input_tokens} + cache_read " + f"{provider.cache_read_tokens} + cache_write {provider.cache_write_tokens}).\n{context}" + ) diff --git a/tests/e2e/suites/usage/test_usage_wrapper.py b/tests/e2e/suites/usage/test_usage_wrapper.py new file mode 100644 index 00000000..41cb4a7a --- /dev/null +++ b/tests/e2e/suites/usage/test_usage_wrapper.py @@ -0,0 +1,394 @@ +"""DTU-backed tests for per-turn usage on the Python wrapper SDK's terminal events. + +Contract under test, on ``amplifier_agent_py``: + + ResultEvent gains session_id, turn_id, exit_code, usage, stderr_tail + ErrorEvent gains session_id, turn_id, exit_code, usage + Usage carries input_tokens, output_tokens, cache_read_tokens, + cache_write_tokens, cost_usd (Decimal | None) + + spawn_agent / spawn_agent_sync gain ``stderr_tail_bytes: int | None = 4096`` + positive int -> the last N BYTES of stderr, never split mid-codepoint + None -> the entire stderr buffer + 0 -> disabled; the field is None + +Today ``ResultEvent`` carries only ``text``, there is no ``Usage`` type, and +``stderr_tail`` is (a) present on ``ErrorEvent`` alone and (b) sliced as a ``str``, so +``STDERR_TAIL_BYTES`` counts CHARACTERS. Every case here is therefore +``xfail(strict=True)``; strict turns the moment the feature lands into a hard failure +that says "remove the marker". + +Each case drives one real turn INSIDE the DTU via ``fixtures/usage_driver.py``, which +reports the terminal event as a single JSON line. The driver never asserts and never +raises on a missing attribute -- it records which contract attributes were absent, so a +failure here reads as "ResultEvent has no 'usage' attribute" rather than as a traceback +from inside the container. +""" + +from __future__ import annotations + +import json +import shlex +import uuid +from collections.abc import Callable +from dataclasses import dataclass +from typing import Any + +import pytest +from framework import dtu + +from suites.usage.events_oracle import read_provider_usage + +pytestmark = pytest.mark.dtu + +# Host config seeded by DTU provisioning: anthropic / claude-sonnet-5 / approval yes. +# Same literal as the envelope half of this suite and as conftest.HOST_CONFIG; spelled +# out here rather than imported from conftest, because a conftest module is pytest's to +# load and importing one by name is not a supported entry point. +HOST_CONFIG = "/root/e2e/host-config.json" + +# Short and tool-free, so the turn stays a single provider call in a single session. +PROMPT = "Reply with the single word: pong" + +# Long enough that the engine's ndjson stderr stream comfortably exceeds the 4096-byte +# default cap, which is what makes "full buffer" and "default cap" distinguishable. +NOISY_PROMPT = "Write three paragraphs about the history of the banana trade." + +# Non-ASCII on stderr is only reachable through the TEXT display: the ndjson renderer +# serializes with json.dumps' default ensure_ascii=True, so every non-ASCII codepoint +# leaves the process as an ASCII \uXXXX escape and a bytes-vs-chars bug would be +# invisible. The text renderer writes the reply verbatim to stderr as UTF-8. +NON_ASCII_PROMPT = "Write three paragraphs in Japanese about the history of the banana trade. Reply in Japanese only." + +# Byte cap used by the bytes-not-chars case. Small enough that a character-counting +# implementation overshoots it by roughly 3x on Japanese text, and large enough that the +# tail is unambiguously inside the multibyte region rather than in the ASCII framing. +NON_ASCII_TAIL_BYTES = 512 + + +def _session_id(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _run_driver( + dtu_id: str, + driver: str, + engine_bin: str, + *, + session_id: str, + prompt: str, + display_mode: str | None = None, + stderr_tail_bytes: str | None = None, +) -> dict[str, Any]: + """Run one wrapper-driven turn in the DTU and return the driver's JSON report.""" + argv = [ + "python3", + driver, + "--session-id", + session_id, + "--prompt", + prompt, + "--config", + HOST_CONFIG, + "--engine-bin", + engine_bin, + ] + if display_mode is not None: + argv += ["--display-mode", display_mode] + if stderr_tail_bytes is not None: + argv += ["--stderr-tail-bytes", stderr_tail_bytes] + + # A login shell so /etc/profile.d exports (provider credential, TLS/proxy vars the + # DTU installs) are in the environment the driver hands to the engine. + result = dtu.exec_json(dtu_id, ["bash", "-lc", " ".join(shlex.quote(part) for part in argv)]) + + stdout = result.get("stdout", "") + lines = [line for line in stdout.splitlines() if line.strip()] + if not lines: + raise AssertionError( + "the wrapper driver produced no report on stdout.\n" + f"exit_code: {result.get('exit_code')}\nstderr:\n{result.get('stderr', '')}" + ) + + try: + return json.loads(lines[-1]) + except json.JSONDecodeError as exc: + raise AssertionError( + "the wrapper driver's last stdout line was not JSON.\n" + f"exit_code: {result.get('exit_code')}\nstdout:\n{stdout}\nstderr:\n{result.get('stderr', '')}" + ) from exc + + +def _require_ok(report: dict[str, Any]) -> None: + """Fail with the driver's own diagnosis when it could not complete a turn.""" + if report.get("ok"): + return + + kind = report.get("error_kind") + if kind == "spawn_kwarg_rejected": + pytest.fail( + "spawn_agent_sync() rejected the 'stderr_tail_bytes' option -- the session " + "option does not exist on this build of amplifier_agent_py.\n" + f"driver error: {report.get('error')}" + ) + pytest.fail( + f"the wrapper driver could not complete a turn ({kind}): {report.get('error')}\n{report.get('traceback', '')}" + ) + + +def _require_attrs(report: dict[str, Any], *names: str) -> None: + """Fail naming the exact contract attributes the terminal event is missing.""" + missing = [name for name in names if name in (report.get("missing_attrs") or [])] + if missing: + pytest.fail( + f"{report.get('event_type')!r} event is missing {missing} -- the wrapper does not " + "surface these yet.\n" + f"attributes absent on the event: {report.get('missing_attrs')}" + ) + + +# --------------------------------------------------------------------------- # +# usage + identity on the terminal event +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def wrapper_turn(dtu_id: str, usage_driver: str, engine_bin: str) -> tuple[str, dict[str, Any]]: + """Drive ONE wrapper turn and share its report across the usage and identity cases. + + Shared rather than run twice because both cases interrogate the same terminal + event; running the turn once also means the identity assertions and the usage + assertions are describing the same provider call, not two that happened to be + similar. + + Deliberately does NO asserting on the report's contents. A missing ``usage`` + attribute is the expected red state, and raising it here would surface as a setup + ERROR (which ``xfail`` does not cover) instead of a test failure. + """ + session_id = _session_id("usage-wrap") + report = _run_driver( + dtu_id, + usage_driver, + engine_bin, + session_id=session_id, + prompt=PROMPT, + ) + return session_id, report + + +def test_usage_result_event_carries_usage(dtu_id: str, wrapper_turn: tuple[str, dict[str, Any]]) -> None: + """``ResultEvent.usage`` is populated and reports the real per-turn totals. + + "Real" is pinned against the same independent oracle the envelope suite uses: the + provider's own per-call usage recorded in the session's + ``context-intelligence/events.jsonl``. Comparing the wrapper's numbers to that + rather than to the envelope it parsed is deliberate -- an envelope-to-wrapper + comparison only proves the wrapper copied a field, and would agree perfectly with + the envelope while both reported zero. + """ + session_id, report = wrapper_turn + _require_ok(report) + _require_attrs(report, "usage") + + assert report.get("event_type") == "result", ( + f"expected a result event, got {report.get('event_type')!r}: {report.get('message') or report.get('code')}" + ) + + usage = report.get("usage") + assert usage is not None, ( + "ResultEvent.usage is None on a turn that completed successfully. A turn that " + "reached the provider always has usage to report." + ) + + missing_usage_attrs = report.get("usage_missing_attrs") or [] + assert not missing_usage_attrs, ( + f"Usage is missing {missing_usage_attrs}. The contract is " + "input_tokens / output_tokens / cache_read_tokens / cache_write_tokens / cost_usd." + ) + + provider = read_provider_usage(dtu_id, session_id) + context = ( + f"session={session_id}\n" + f"wrapper usage: {json.dumps(usage, sort_keys=True)}\n" + f"provider totals: new_input={provider.input_tokens} output={provider.output_tokens} " + f"cache_read={provider.cache_read_tokens} cache_write={provider.cache_write_tokens} " + f"charged_input={provider.charged_input}" + ) + + assert usage.get("output_tokens") == provider.output_tokens, ( + f"Usage.output_tokens ({usage.get('output_tokens')}) != provider-reported output " + f"tokens ({provider.output_tokens}).\n{context}" + ) + assert usage.get("input_tokens") == provider.charged_input, ( + f"Usage.input_tokens ({usage.get('input_tokens')}) != the provider's CHARGED input " + f"total ({provider.charged_input}). Usage.input_tokens mirrors the envelope's " + f"tokensIn, which is new input + cache reads + cache writes.\n{context}" + ) + + +def test_usage_result_event_identity(wrapper_turn: tuple[str, dict[str, Any]]) -> None: + """The terminal event says which session and turn it belongs to, and how it exited. + + Without these a host holding several concurrent turns cannot attribute a result to + the request that produced it, and usage numbers with no turn to attach to are not + usable for accounting. + """ + session_id, report = wrapper_turn + _require_ok(report) + _require_attrs(report, "session_id", "turn_id", "exit_code") + + assert report.get("session_id") == session_id, ( + f"ResultEvent.session_id is {report.get('session_id')!r}, expected the requested session id {session_id!r}." + ) + + turn_id = report.get("turn_id") + assert isinstance(turn_id, str) and turn_id, f"ResultEvent.turn_id must be a non-empty string, got {turn_id!r}." + + assert report.get("exit_code") == 0, ( + f"ResultEvent.exit_code is {report.get('exit_code')!r}; a successful turn exits 0." + ) + + +# --------------------------------------------------------------------------- # +# stderr_tail_bytes +# --------------------------------------------------------------------------- # + + +@dataclass(frozen=True) +class TailCase: + """One ``stderr_tail_bytes`` scenario. + + ``arg`` is what reaches the driver's ``--stderr-tail-bytes`` flag: ``None`` means + the flag is omitted entirely, so the SDK's default applies rather than an explicitly + passed value. ``check`` receives the driver's report. + """ + + arg: str | None + prompt: str + display_mode: str | None + check: Callable[[dict[str, Any]], None] + + +def _check_default(report: dict[str, Any]) -> None: + """Default (4096): the tail is present and no larger than the cap, in BYTES.""" + tail = report.get("stderr_tail") + assert tail is not None, ( + "stderr_tail is None on a turn whose engine wrote a substantial ndjson stream to " + "stderr. The default (4096) must produce a tail, not suppress it." + ) + size = report["stderr_tail_utf8_len"] + assert size <= 4096, f"stderr_tail is {size} bytes, above the 4096-byte default cap." + assert size > 0, "stderr_tail is present but empty." + + +def _check_full(report: dict[str, Any]) -> None: + """``None``: the ENTIRE buffer, which for this prompt exceeds the default cap.""" + tail = report.get("stderr_tail") + assert tail is not None, "stderr_tail_bytes=None must return the full stderr buffer, not None." + size = report["stderr_tail_utf8_len"] + assert size > 4096, ( + f"stderr_tail is {size} bytes with stderr_tail_bytes=None. This turn ran with " + "--display ndjson precisely so stderr would exceed the 4096-byte default; a value " + "at or below it means the buffer was still being truncated." + ) + + +def _check_bounded(report: dict[str, Any]) -> None: + """``512``: exactly 512 bytes retained.""" + tail = report.get("stderr_tail") + assert tail is not None, "stderr_tail_bytes=512 must return a tail, not None." + size = report["stderr_tail_utf8_len"] + assert size == 512, ( + f"stderr_tail is {size} bytes with stderr_tail_bytes=512, expected exactly 512. " + "The engine's ndjson stderr for this turn is far longer than 512 bytes and is " + "ASCII, so no codepoint-boundary trim applies." + ) + + +def _check_disabled(report: dict[str, Any]) -> None: + """``0``: the field is None, even though stderr had plenty to report.""" + assert report.get("stderr_tail") is None, ( + f"stderr_tail_bytes=0 must disable capture, but stderr_tail is {report['stderr_tail_utf8_len']} bytes." + ) + + +def _check_bytes_not_chars(report: dict[str, Any]) -> None: + """Non-ASCII stderr: the cap counts BYTES, and the tail still decodes cleanly. + + This is the whole point of the fix. Slicing a ``str`` counts characters, so on + Japanese text a 512-"byte" cap yields roughly 1536 real bytes. Slicing bytes without + respecting codepoint boundaries yields a leading U+FFFD instead. The contract is + both: at most N bytes, and no replacement character. + """ + tail = report.get("stderr_tail") + assert tail is not None, "stderr_tail is None; the non-ASCII case has nothing to measure." + + assert not report["stderr_tail_is_ascii"], ( + "the captured stderr tail is pure ASCII, so this case cannot distinguish a " + "byte cap from a character cap. The turn was asked for a Japanese reply under " + "--display text specifically so the reply would reach stderr as multibyte UTF-8." + ) + + size = report["stderr_tail_utf8_len"] + chars = report["stderr_tail_char_len"] + assert size <= NON_ASCII_TAIL_BYTES, ( + f"stderr_tail is {size} BYTES ({chars} characters) with " + f"stderr_tail_bytes={NON_ASCII_TAIL_BYTES}. The cap is specified in bytes; this " + "is a character count applied to a multibyte string." + ) + assert not report["stderr_tail_has_replacement_char"], ( + "stderr_tail contains U+FFFD, so the byte slice landed mid-codepoint and was " + "decoded with errors='replace'. A byte-bounded tail must back up to a codepoint " + "boundary rather than emit a broken character." + ) + + +_TAIL_CASES = [ + pytest.param( + TailCase(arg=None, prompt=NOISY_PROMPT, display_mode="ndjson", check=_check_default), + id="usage-stderr-tail-default", + ), + pytest.param( + TailCase(arg="none", prompt=NOISY_PROMPT, display_mode="ndjson", check=_check_full), + id="usage-stderr-tail-full", + ), + pytest.param( + TailCase(arg="512", prompt=NOISY_PROMPT, display_mode="ndjson", check=_check_bounded), + id="usage-stderr-tail-bounded", + ), + pytest.param( + TailCase(arg="0", prompt=NOISY_PROMPT, display_mode="ndjson", check=_check_disabled), + id="usage-stderr-tail-disabled", + ), + pytest.param( + TailCase( + arg=str(NON_ASCII_TAIL_BYTES), + prompt=NON_ASCII_PROMPT, + display_mode="text", + check=_check_bytes_not_chars, + ), + id="usage-stderr-tail-bytes-not-chars", + ), +] + + +@pytest.mark.parametrize("case", _TAIL_CASES) +def test_usage_stderr_tail(dtu_id: str, usage_driver: str, engine_bin: str, case: TailCase) -> None: + """``stderr_tail_bytes`` bounds the terminal event's stderr tail in real bytes.""" + report = _run_driver( + dtu_id, + usage_driver, + engine_bin, + session_id=_session_id("usage-tail"), + prompt=case.prompt, + display_mode=case.display_mode, + stderr_tail_bytes=case.arg, + ) + _require_ok(report) + _require_attrs(report, "stderr_tail") + + assert report.get("event_type") == "result", ( + f"expected a result event, got {report.get('event_type')!r}: {report.get('message') or report.get('code')}" + ) + + case.check(report) diff --git a/wrappers/python-py/README.md b/wrappers/python-py/README.md index 224940ba..5334e2f0 100644 --- a/wrappers/python-py/README.md +++ b/wrappers/python-py/README.md @@ -89,11 +89,11 @@ with spawn_agent_sync(session_id="demo-2") as handle: ## Protocol version pinning -This wrapper version is pinned to **wire protocol 0.3.0**. On `spawn_agent()`, the wrapper runs `amplifier-agent version --json` and compares the engine's reported protocol version against `PROTOCOL_VERSION_REQUIRED_BY_WRAPPER`. Mismatch raises `AaaError(protocol_version_mismatch)` unless you pass `allow_protocol_skew=True`. +This wrapper version is pinned to **wire protocol 0.4.0**. On `spawn_agent()`, the wrapper runs `amplifier-agent version --json` and compares the engine's reported protocol version against `PROTOCOL_VERSION_REQUIRED_BY_WRAPPER`. Mismatch raises `AaaError(protocol_version_mismatch)` unless you pass `allow_protocol_skew=True`. | Wrapper version | Required engine protocol | |---|---| -| `amplifier-agent-py` 0.3.x | `amplifier-agent` reporting protocol `0.3.0` | +| `amplifier-agent-py` 0.3.x | `amplifier-agent` reporting protocol `0.4.0` | Wrapper version tracks the wire protocol, not the engine version. Multiple engine versions can speak the same protocol. @@ -159,7 +159,7 @@ Common codes: ## Project status -Version 0.3.0 corresponds to wire protocol 0.3.0. The wrapper is a clean port of the TypeScript wrapper at the same protocol revision. Conformance is enforced by the shared fixture suite under `wrappers/conformance/`. +Version 0.3.0 corresponds to wire protocol 0.4.0. The wrapper is a clean port of the TypeScript wrapper at the same protocol revision. Conformance is enforced by the shared fixture suite under `wrappers/conformance/`. ## License diff --git a/wrappers/python-py/examples/README.md b/wrappers/python-py/examples/README.md index 9f70c570..b6ba726a 100644 --- a/wrappers/python-py/examples/README.md +++ b/wrappers/python-py/examples/README.md @@ -42,7 +42,7 @@ Expected output: ``` OK binary discovered: /Users/you/.local/bin/amplifier-agent -OK protocol version: wrapper=0.3.0 engine=0.3.0 +OK protocol version: wrapper=0.4.0 engine=0.4.0 OK engine version: 0.6.0 OK spawn_agent() returned a SessionHandle without launching the engine subprocess. ``` diff --git a/wrappers/python-py/src/amplifier_agent_py/__init__.py b/wrappers/python-py/src/amplifier_agent_py/__init__.py index 487aeae5..6a3defa4 100644 --- a/wrappers/python-py/src/amplifier_agent_py/__init__.py +++ b/wrappers/python-py/src/amplifier_agent_py/__init__.py @@ -35,6 +35,7 @@ from .run_output_parser import STDERR_TAIL_BYTES as STDERR_TAIL_BYTES from .run_output_parser import SubprocessOutcome as SubprocessOutcome from .run_output_parser import parse_run_output as parse_run_output +from .run_output_parser import tail_stderr_bytes as tail_stderr_bytes from .session import DEFAULT_TIMEOUT_MS as DEFAULT_TIMEOUT_MS from .session import SessionHandle as SessionHandle from .session import SessionHandleParams as SessionHandleParams @@ -55,6 +56,7 @@ from .types import McpServerConfig as McpServerConfig from .types import NotificationEvent as NotificationEvent from .types import ResultEvent as ResultEvent +from .types import Usage as Usage from .version import VersionCheckFail as VersionCheckFail from .version import VersionCheckOk as VersionCheckOk from .version import VersionCheckResult as VersionCheckResult @@ -86,6 +88,7 @@ "Severity", "SubprocessOutcome", "SyncSessionHandle", + "Usage", "VersionCheckFail", "VersionCheckOk", "VersionCheckResult", @@ -100,4 +103,5 @@ "resolve_mcp_config_path", "spawn_agent", "spawn_agent_sync", + "tail_stderr_bytes", ] diff --git a/wrappers/python-py/src/amplifier_agent_py/_api.py b/wrappers/python-py/src/amplifier_agent_py/_api.py index 7b6612a9..64a6ed5b 100644 --- a/wrappers/python-py/src/amplifier_agent_py/_api.py +++ b/wrappers/python-py/src/amplifier_agent_py/_api.py @@ -19,6 +19,7 @@ from .argv_builder import ApprovalMode, DisplayMode from .errors import AaaError +from .run_output_parser import STDERR_TAIL_BYTES from .session import SessionHandle, SessionHandleParams from .spawn import DEFAULT_ALLOWLIST, build_env, probe_engine_version, resolve_binary_path from .types import DisplayEvent, McpServerConfig @@ -27,7 +28,11 @@ #: The protocol version this wrapper requires. Forwarded to the engine via #: ``--protocol-version`` on every ``submit()`` and checked at #: ``spawn_agent()`` time against the engine's reported protocol version. -PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.3.0" +#: +#: 0.4.0 added the envelope's per-turn usage block (``cacheReadTokens``, +#: ``cacheWriteTokens``, ``costUsd``, and real ``tokensIn``/``tokensOut``) that +#: ``ResultEvent.usage`` / ``ErrorEvent.usage`` surface. +PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.4.0" async def spawn_agent( @@ -45,6 +50,7 @@ async def spawn_agent( timeout_ms: int | None = None, config_path: str | None = None, allow_protocol_skew: bool = False, + stderr_tail_bytes: int | None = STDERR_TAIL_BYTES, _binary_resolver: Callable[[], str] | None = None, _engine_version_probe: Callable[[], Any] | None = None, ) -> SessionHandle: @@ -82,6 +88,13 @@ async def spawn_agent( timeout_ms: Per-submit wall-clock cap (None / 0 disables). config_path: Path to engine host config file (``--config``). allow_protocol_skew: Skip the wrapper-side version probe. + stderr_tail_bytes: Byte cap on ``stderr_tail`` for the terminal + ``ResultEvent`` / ``ErrorEvent``. A positive int + keeps the last N UTF-8 BYTES (never splitting a + codepoint), ``None`` keeps the ENTIRE stderr + buffer, and ``0`` disables capture so the field + stays ``None``. Defaults to + ``STDERR_TAIL_BYTES`` (4096). Returns: ``SessionHandle`` โ€” call ``submit()`` to drive a single turn. @@ -199,5 +212,6 @@ async def spawn_agent( display_on_event=display_on_event, engine_version=engine_version, bundle_digest=engine_bundle_digest, + stderr_tail_bytes=stderr_tail_bytes, ) ) diff --git a/wrappers/python-py/src/amplifier_agent_py/run_output_parser.py b/wrappers/python-py/src/amplifier_agent_py/run_output_parser.py index a8747c8c..ec49d1d3 100644 --- a/wrappers/python-py/src/amplifier_agent_py/run_output_parser.py +++ b/wrappers/python-py/src/amplifier_agent_py/run_output_parser.py @@ -12,22 +12,32 @@ Rule 2 โ€” envelope absent / unparseable / partial โ†’ synthesize an error event from exit code and stderr tail. Partial JSON is NOT half-parsed (belt-and-suspenders): if any required ยง4.1 field - is missing, the envelope is treated as unparseable. + is missing, the envelope is treated as unparseable. The turn id + is unknowable here (the engine assigns it), but the SESSION id is + the caller's own -- pass it as ``fallback_session_id`` and it is + reported rather than dropped. -``stderr_tail`` is truncated to ``STDERR_TAIL_BYTES`` (4096) on synthesized -paths; on the envelope path it is taken verbatim from the engine. +On the Rule 1 path the envelope is now *read*, not merely shape-checked: +``sessionId``, ``turnId`` and the ``metadata`` usage block are surfaced on the +terminal event. The wrapper performs no arithmetic of its own -- the engine's +``UsageAccumulator`` already summed the turn, so re-summing here would +double-count. + +``stderr_tail`` is bounded by ``stderr_tail_bytes`` in real UTF-8 BYTES (see +``tail_stderr_bytes``); ``STDERR_TAIL_BYTES`` (4096) is the default. """ from __future__ import annotations import json from dataclasses import dataclass +from decimal import Decimal, InvalidOperation from typing import Any, cast from .errors import Classification -from .types import DisplayEvent, ErrorEvent, ResultEvent +from .types import DisplayEvent, ErrorEvent, ResultEvent, Usage -#: Maximum stderr_tail length retained on synthesized engine errors. +#: Default cap on ``stderr_tail``, in BYTES of UTF-8. STDERR_TAIL_BYTES = 4096 #: Maximum stdout snippet included in ``envelope_missing`` messages. @@ -35,6 +45,17 @@ _VALID_CLASSIFICATIONS: frozenset[str] = frozenset({"transport", "protocol", "engine", "approval", "unknown"}) +#: The envelope ``metadata`` keys that make up a usage report. Presence of at +#: least one of them is what distinguishes "the engine reported usage" from +#: "this engine predates protocol 0.4.0 and reported none". +_USAGE_KEYS: tuple[str, ...] = ( + "tokensIn", + "tokensOut", + "cacheReadTokens", + "cacheWriteTokens", + "costUsd", +) + @dataclass(frozen=True, kw_only=True) class SubprocessOutcome: @@ -45,13 +66,47 @@ class SubprocessOutcome: exit_code: int -def _tail_stderr(stderr: str) -> str | None: - """Keep the last ``STDERR_TAIL_BYTES`` chars of ``stderr`` or ``None``.""" - if not stderr: +def tail_stderr_bytes(text: str, limit: int | None = STDERR_TAIL_BYTES) -> str | None: + """Return the last ``limit`` BYTES of ``text``, never splitting a codepoint. + + The cap is expressed in bytes because that is what a host budgeting a log + line or a payload actually cares about; a character count is meaningless + for that purpose the moment stderr contains non-ASCII. + + ``limit`` semantics: + + * a positive int -- at most that many UTF-8 bytes, taken from the END. + * ``None`` -- the entire buffer, uncapped. + * ``0`` or less -- capture disabled; returns ``None``. + + An empty ``text`` always yields ``None``: there is nothing to report. + + Boundary safety: after slicing the encoded buffer, leading UTF-8 + continuation bytes (``0b10xxxxxx``) are dropped so the slice begins on a + lead byte. The result therefore decodes strictly -- no ``U+FFFD``, no + ``UnicodeDecodeError`` -- at the cost of up to 3 bytes fewer than ``limit``. + Returning slightly less than the cap is correct; returning a broken + character is not. + """ + if not text: return None - if len(stderr) <= STDERR_TAIL_BYTES: - return stderr - return stderr[-STDERR_TAIL_BYTES:] + if limit is None: + return text + if limit <= 0: + return None + + raw = text.encode("utf-8") + if len(raw) <= limit: + return text + + window = raw[-limit:] + start = 0 + # A UTF-8 continuation byte matches 0b10xxxxxx; a lead byte never does. The + # source is a valid str, so at most 3 continuation bytes can precede the + # first lead byte in the window. + while start < len(window) and (window[start] & 0xC0) == 0x80: + start += 1 + return window[start:].decode("utf-8") def _is_shape_valid(parsed: Any) -> bool: @@ -77,10 +132,83 @@ def _is_shape_valid(parsed: Any) -> bool: return isinstance(err.get("code"), str) -def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: +def _to_int(value: Any) -> int: + """Coerce a wire token count to an int, defaulting to 0. + + Deliberately total: a malformed count must not be able to turn a completed + turn into an exception on the host's side. + """ + if value is None: + return 0 + try: + return int(value) + except (TypeError, ValueError): + return 0 + + +def _to_decimal(value: Any) -> Decimal | None: + """Parse the wire ``costUsd`` string into a ``Decimal``, or ``None``. + + ``str()`` first, always: ``Decimal(0.1)`` captures the binary float's error, + ``Decimal("0.1")`` does not. A cost that will not parse is reported as + absent rather than as zero. + """ + if value is None: + return None + try: + return Decimal(str(value)) + except (InvalidOperation, TypeError, ValueError): + return None + + +def _usage_from_metadata(metadata: Any) -> Usage | None: + """Read the envelope's ``metadata`` usage block into a ``Usage``. + + Returns ``None`` when the metadata carries none of the usage keys at all -- + an engine older than protocol 0.4.0 never reported them, and claiming zeroes + on its behalf would be a fabricated number rather than an absent one. + """ + if not isinstance(metadata, dict): + return None + meta = cast(dict[str, Any], metadata) + if not any(key in meta for key in _USAGE_KEYS): + return None + + return Usage( + # Mirrors the envelope verbatim: tokensIn is the CHARGED total the + # engine already computed. No wrapper-side arithmetic. + input_tokens=_to_int(meta.get("tokensIn")), + output_tokens=_to_int(meta.get("tokensOut")), + cache_read_tokens=_to_int(meta.get("cacheReadTokens")), + cache_write_tokens=_to_int(meta.get("cacheWriteTokens")), + cost_usd=_to_decimal(meta.get("costUsd")), + ) + + +def parse_run_output( + outcome: SubprocessOutcome, + *, + stderr_tail_bytes: int | None = STDERR_TAIL_BYTES, + fallback_session_id: str | None = None, +) -> DisplayEvent: """Parse a subprocess outcome into a single ``DisplayEvent``. See module docstring for precedence rules. + + Args: + outcome: stdout / stderr / exit code of the finished subprocess. + stderr_tail_bytes: byte cap for ``stderr_tail`` on the returned event. + Positive int caps to that many UTF-8 bytes, ``None`` keeps the whole + buffer, ``0`` disables the field. The cap applies to whatever ends + up in ``stderr_tail``, including a tail the engine supplied in the + envelope, so ``0`` really does mean "do not give me stderr". + fallback_session_id: the caller's own session id, reported on the + SYNTHESIZED (Rule 2) events only. Never overrides the envelope: + on the Rule 1 path the envelope's ``sessionId`` is authoritative + and this argument is ignored. ``None`` (the default) preserves + the previous behaviour of leaving the field unset -- a caller with + no session id to offer, such as a host post-parsing a captured + payload, is not made to invent one. """ trimmed = outcome.stdout.strip() @@ -94,10 +222,23 @@ def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: # Rule 1 โ€” envelope parseable per ยง4.1 โ†’ envelope wins. if parsed is not None and _is_shape_valid(parsed): env = cast(dict[str, Any], parsed) + session_id = cast(str, env["sessionId"]) + turn_id = cast(str, env["turnId"]) + usage = _usage_from_metadata(env.get("metadata")) err = env.get("error") if err is None: - return ResultEvent(text=cast(str, env["reply"])) + return ResultEvent( + text=cast(str, env["reply"]), + session_id=session_id, + turn_id=turn_id, + # Informational per SC-D: the envelope already decided the + # outcome. Reported as observed so a host can still see a + # post-flush crash (a result event with a non-zero exit). + exit_code=outcome.exit_code, + usage=usage, + stderr_tail=tail_stderr_bytes(outcome.stderr, stderr_tail_bytes), + ) # Failure path โ€” populate from the envelope's error fields. err_dict = cast(dict[str, Any], err) @@ -112,10 +253,8 @@ def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: message = message_raw if isinstance(message_raw, str) else cast(str, err_dict["code"]) envelope_tail = err_dict.get("stderrTail") - if isinstance(envelope_tail, str): - stderr_tail: str | None = envelope_tail - else: - stderr_tail = _tail_stderr(outcome.stderr) + source_tail = envelope_tail if isinstance(envelope_tail, str) else outcome.stderr + stderr_tail = tail_stderr_bytes(source_tail, stderr_tail_bytes) return ErrorEvent( code=cast(str, err_dict["code"]), @@ -125,10 +264,20 @@ def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: message=message, stderr_tail=stderr_tail, retryable=False, + session_id=session_id, + turn_id=turn_id, + exit_code=outcome.exit_code, + usage=usage, ) # Rule 2 โ€” envelope absent or unparseable โ†’ synthesize from exit + stderr. - stderr_tail = _tail_stderr(outcome.stderr) + # No envelope means no turnId and no usage to report: the engine assigns the + # turn id and nothing came back, so inventing one would be a fabrication. + # The SESSION id is different -- the caller minted it and passed it in as + # ``fallback_session_id``, so a host correlating this failure against its own + # records still gets the handle it already knows. The exit code remains + # load-bearing for the code/classification split below. + stderr_tail = tail_stderr_bytes(outcome.stderr, stderr_tail_bytes) if outcome.exit_code == 0: preview = outcome.stdout[:_STDOUT_PREVIEW_BYTES] @@ -144,6 +293,8 @@ def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: ), stderr_tail=stderr_tail, retryable=False, + session_id=fallback_session_id, + exit_code=outcome.exit_code, ) return ErrorEvent( @@ -154,4 +305,6 @@ def parse_run_output(outcome: SubprocessOutcome) -> DisplayEvent: message=f"Engine exited {outcome.exit_code} without emitting a parseable ยง4.1 envelope.", stderr_tail=stderr_tail, retryable=False, + session_id=fallback_session_id, + exit_code=outcome.exit_code, ) diff --git a/wrappers/python-py/src/amplifier_agent_py/session.py b/wrappers/python-py/src/amplifier_agent_py/session.py index 97bfeff2..06324eb2 100644 --- a/wrappers/python-py/src/amplifier_agent_py/session.py +++ b/wrappers/python-py/src/amplifier_agent_py/session.py @@ -41,7 +41,7 @@ from .errors import AaaError from .mcp_spill import cleanup_spill_file, resolve_mcp_config_path from .prompt_spill import cleanup_prompt_spill_file, resolve_prompt_file_path -from .run_output_parser import STDERR_TAIL_BYTES, SubprocessOutcome, parse_run_output +from .run_output_parser import STDERR_TAIL_BYTES, SubprocessOutcome, parse_run_output, tail_stderr_bytes from .types import ( ActivityEvent, DisplayEvent, @@ -64,15 +64,6 @@ _SIGKILL_GRACE_S = 5.0 -def _stderr_tail_of(stderr: str) -> str | None: - """Last ``STDERR_TAIL_BYTES`` chars of ``stderr``, or None if empty.""" - if not stderr: - return None - if len(stderr) <= STDERR_TAIL_BYTES: - return stderr - return stderr[-STDERR_TAIL_BYTES:] - - @dataclass(kw_only=True) class SessionHandleParams: """Parameters for constructing a ``SessionHandle``. @@ -96,6 +87,11 @@ class SessionHandleParams: display_on_event: Callable[[DisplayEvent], None] | None = None engine_version: str = "" bundle_digest: str = "" + #: Byte cap on ``stderr_tail`` for BOTH the terminal ``ResultEvent`` and + #: ``ErrorEvent``. Positive int -> last N UTF-8 BYTES; ``None`` -> the + #: entire buffer; ``0`` -> capture disabled. The 4096 default preserves the + #: historical ``ErrorEvent`` behaviour exactly. + stderr_tail_bytes: int | None = STDERR_TAIL_BYTES @dataclass @@ -212,7 +208,9 @@ def finalize(ev: DisplayEvent) -> None: start_new_session=True, ) except (FileNotFoundError, PermissionError, OSError) as e: - # Spawn-time failure (ENOENT, EACCES) โ†’ typed error. + # Spawn-time failure (ENOENT, EACCES) โ†’ typed error. No engine ran, + # so there is no turn id to report -- but the session id is the + # handle's own, and a host correlating this failure needs it. tail = None yield ErrorEvent( code="spawn_failed", @@ -222,6 +220,7 @@ def finalize(ev: DisplayEvent) -> None: message=f"Failed to spawn engine subprocess ({type(e).__name__}): {e}", stderr_tail=tail, retryable=False, + session_id=self._params.session_id, ) cleanup_spill_file(self._mcp_spill_path) self._mcp_spill_path = None @@ -297,7 +296,13 @@ async def watch_exit() -> None: stdout="".join(stdout_buf), stderr="".join(stderr_buf), exit_code=exit_code, - ) + ), + stderr_tail_bytes=self._params.stderr_tail_bytes, + # Rule 2 synthesis has no envelope to read identity from; hand + # the parser the session id we already hold so the synthesized + # error still carries it. Ignored on the Rule 1 path, where the + # envelope is authoritative. + fallback_session_id=self._params.session_id, ) finalize(ev) @@ -312,7 +317,7 @@ async def watch_timeout() -> None: return if state.finalized: return - tail = _stderr_tail_of("".join(stderr_buf)) + tail = tail_stderr_bytes("".join(stderr_buf), self._params.stderr_tail_bytes) finalize( ErrorEvent( code="engine_hung", @@ -324,6 +329,9 @@ async def watch_timeout() -> None: ), stderr_tail=tail, retryable=False, + # The engine never reported a turn id, but the session id is + # ours -- report it so a hung turn is still correlatable. + session_id=self._params.session_id, ) ) # Fire-and-forget cancel; keep a reference so the task is not diff --git a/wrappers/python-py/src/amplifier_agent_py/sync.py b/wrappers/python-py/src/amplifier_agent_py/sync.py index 9c27ab60..d3d01b5d 100644 --- a/wrappers/python-py/src/amplifier_agent_py/sync.py +++ b/wrappers/python-py/src/amplifier_agent_py/sync.py @@ -32,6 +32,7 @@ from ._api import spawn_agent from .argv_builder import DisplayMode +from .run_output_parser import STDERR_TAIL_BYTES from .session import SessionHandle from .types import DisplayEvent, EngineInfo, McpServerConfig @@ -150,6 +151,7 @@ def spawn_agent_sync( timeout_ms: int | None = None, config_path: str | None = None, allow_protocol_skew: bool = False, + stderr_tail_bytes: int | None = STDERR_TAIL_BYTES, _binary_resolver: Callable[[], str] | None = None, _engine_version_probe: Callable[[], Any] | None = None, ) -> SyncSessionHandle: @@ -178,6 +180,7 @@ def spawn_agent_sync( timeout_ms=timeout_ms, config_path=config_path, allow_protocol_skew=allow_protocol_skew, + stderr_tail_bytes=stderr_tail_bytes, _binary_resolver=_binary_resolver, _engine_version_probe=_engine_version_probe, ) diff --git a/wrappers/python-py/src/amplifier_agent_py/types.py b/wrappers/python-py/src/amplifier_agent_py/types.py index e044de23..a936313a 100644 --- a/wrappers/python-py/src/amplifier_agent_py/types.py +++ b/wrappers/python-py/src/amplifier_agent_py/types.py @@ -11,10 +11,53 @@ from __future__ import annotations from dataclasses import dataclass, field +from decimal import Decimal from typing import Any, Literal from .errors import Classification, Severity +# --------------------------------------------------------------------------- +# Usage (mirror `Usage` in wrappers/typescript/src/session.ts) +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, kw_only=True) +class Usage: + """Per-turn token and cost accounting, as reported by the engine. + + Read straight off the ยง4.1 envelope's ``metadata`` block. The wrapper does + NOT sum anything: the engine's ``UsageAccumulator`` already folded every + ``usage`` display event of the turn (including sub-agent LLM calls) into + these totals, so re-summing wrapper-side would double-count. + + Attributes + ---------- + input_tokens: + Input tokens **charged**, mirroring the envelope's ``tokensIn``. This + is new input + cache reads + cache writes; the model saw all three as + input and the split is a billing distinction. A host that wants the + new-only figure derives it as + ``input_tokens - cache_read_tokens - cache_write_tokens``. + output_tokens: + Output tokens (envelope ``tokensOut``). + cache_read_tokens: + Input tokens served from the provider's prompt cache. + cache_write_tokens: + Input tokens written into the provider's prompt cache. + cost_usd: + Turn cost as a ``Decimal``, parsed from the envelope's decimal + ``costUsd`` STRING. Never a float -- binary floats accumulate drift the + moment a host sums them. ``None`` when no provider reported a cost; + that is not the same claim as ``Decimal("0")``. + """ + + input_tokens: int + output_tokens: int + cache_read_tokens: int + cache_write_tokens: int + cost_usd: Decimal | None = None + + # --------------------------------------------------------------------------- # DisplayEvent variants (mirror wrappers/typescript/src/session.ts) # --------------------------------------------------------------------------- @@ -37,15 +80,59 @@ class ActivityEvent: @dataclass(frozen=True, kw_only=True) class ResultEvent: - """Yielded once when the subprocess emits a successful ยง4.1 envelope.""" + """Yielded once when the subprocess emits a successful ยง4.1 envelope. + + ``session_id``, ``turn_id`` and ``exit_code`` are never optional here: a + ``ResultEvent`` exists only on the envelope path, so the identity fields the + envelope carries are always known. (``ErrorEvent`` can be synthesized with + no envelope at all, which is why its equivalents are nullable.) + + ``usage`` is ``None`` only when the envelope's ``metadata`` carried no usage + keys whatsoever -- an engine older than protocol 0.4.0. ``None`` means "not + reported", which is a different claim from a populated ``Usage`` reading + zero (a turn that made no LLM call really did spend nothing). + + ``stderr_tail`` holds the last ``stderr_tail_bytes`` BYTES of the engine's + stderr, or the whole buffer when that option is ``None``, or ``None`` when + it is ``0`` or stderr was empty. + """ text: str + session_id: str + turn_id: str + exit_code: int + usage: Usage | None = None + stderr_tail: str | None = None type: Literal["result"] = "result" @dataclass(frozen=True, kw_only=True) class ErrorEvent: - """Yielded once when the subprocess errors, hangs, or fails to spawn.""" + """Yielded once when the subprocess errors, hangs, or fails to spawn. + + ``session_id`` / ``turn_id`` / ``exit_code`` / ``usage`` are populated from + the ยง4.1 envelope when one was parsed. + + On the synthesized (Rule 2) paths -- envelope absent, unparseable, spawn + failure, or hang -- there is no envelope to read them from, and the fields + split by who knows the answer: + + * ``session_id`` IS populated whenever the wrapper itself knows it, which + is every failure raised through a ``SessionHandle``: the handle was given + the session id at construction time, so a host correlating the failure + gets the same identifier it passed in. It is ``None`` only when + ``parse_run_output`` is called directly without a + ``fallback_session_id``. + * ``turn_id`` is ``None``. The engine assigns turn ids and no envelope came + back, so the wrapper genuinely does not know it and will not invent one. + * ``exit_code`` is present on the parser's Rule 2 paths (the process did + exit) and ``None`` on spawn failure and hang, where it never did. + * ``usage`` is ``None``: only the envelope reports it. + + ``usage`` on the failure path is not a duplicate report: nothing else on the + failure path carries usage, so a turn that burned tokens and then failed + would otherwise spend them invisibly. + """ code: str classification: Classification @@ -54,6 +141,10 @@ class ErrorEvent: message: str retryable: bool stderr_tail: str | None = None + session_id: str | None = None + turn_id: str | None = None + exit_code: int | None = None + usage: Usage | None = None type: Literal["error"] = "error" diff --git a/wrappers/python-py/src/amplifier_agent_py/version.py b/wrappers/python-py/src/amplifier_agent_py/version.py index 70ee8a37..a8ed86f3 100644 --- a/wrappers/python-py/src/amplifier_agent_py/version.py +++ b/wrappers/python-py/src/amplifier_agent_py/version.py @@ -43,7 +43,7 @@ def check_protocol_version( Args: wrapper: The protocol version compiled into the wrapper - (e.g. ``"0.3.0"``). + (e.g. ``"0.4.0"``). engine: The protocol version reported by the engine binary. allow_skew: If True, bypass the version check and always return ok=True. diff --git a/wrappers/typescript/dist/argv-builder.d.ts b/wrappers/typescript/dist/argv-builder.d.ts index 2a8c7b4f..f3093e89 100644 --- a/wrappers/typescript/dist/argv-builder.d.ts +++ b/wrappers/typescript/dist/argv-builder.d.ts @@ -27,7 +27,7 @@ export interface AssembleArgvInput { * Defaults to undefined, so callers that never spill are unaffected. */ promptFile?: string; - /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ + /** Protocol version the wrapper speaks (e.g. "0.4.0"). */ protocolVersion: string; /** When true, emit `--resume` instead of `--fresh`. */ resume?: boolean; diff --git a/wrappers/typescript/dist/index.d.ts b/wrappers/typescript/dist/index.d.ts index 0d195b6c..bc7666cf 100644 --- a/wrappers/typescript/dist/index.d.ts +++ b/wrappers/typescript/dist/index.d.ts @@ -8,7 +8,7 @@ * at spawn-time** โ€” the engine is launched per `submit()` (amendment ยง5.2). */ export { AaaError, SessionHandle, DEFAULT_TIMEOUT_MS } from "./session.js"; -export type { DisplayEvent, EngineInfo, SessionHandleParams, } from "./session.js"; +export type { DisplayEvent, EngineInfo, SessionHandleParams, Usage, } from "./session.js"; export type { ApprovalResponse } from "./approval.js"; export type { EngineVersionPayload } from "./spawn.js"; /** @public */ @@ -36,9 +36,9 @@ export { checkProtocolVersion } from "./version.js"; /** @public */ export type { VersionCheckResult, VersionCheckOk, VersionCheckFail, CheckProtocolVersionOptions, } from "./version.js"; /** @public */ -export { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; +export { parseRunOutput, tailStderrBytes, STDERR_TAIL_BYTES, } from "./run-output-parser.js"; /** @public */ -export type { SubprocessOutcome } from "./run-output-parser.js"; +export type { SubprocessOutcome, ParseRunOutputOptions, } from "./run-output-parser.js"; /** @public */ export { makeApprovalHandler } from "./approval.js"; /** @public */ @@ -56,8 +56,12 @@ export type { ChildProcessFactory } from "./session.js"; * checked at `spawnAgent()` time against the engine's reported protocol * version (see Issue #9 โ€” `checkProtocolVersion()` is wired into the init * path so skew fails fast wrapper-side before any subprocess spawn). + * + * 0.4.0 added the envelope's per-turn usage block (`cacheReadTokens`, + * `cacheWriteTokens`, `costUsd`, and real `tokensIn`/`tokensOut`) that the + * `usage` field on the terminal `result` / `error` events surfaces. */ -export declare const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.3.0"; +export declare const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.4.0"; /** Parameters for spawnAgent(). Signature is locked verbatim by design ยง8.2. */ export interface SpawnAgentParams { /** 'burst' reserved; throws AaaError(lifecycle_unsupported) at runtime. */ @@ -182,6 +186,24 @@ export interface SpawnAgentParams { * Mirrors the engine-side `host_config.allowProtocolSkew` knob. */ allowProtocolSkew?: boolean; + /** + * Byte cap on `stderrTail` for the terminal `result` / `error` events. + * + * - a positive number โ€” the last N UTF-8 BYTES of the engine's stderr, + * never split mid-codepoint. Trimming to a codepoint boundary can yield + * slightly fewer than N bytes; that is correct. + * - `null` โ€” the ENTIRE stderr buffer, uncapped. + * - `0` โ€” capture disabled; `stderrTail` is omitted. + * - omitted โ€” `STDERR_TAIL_BYTES` (4096) applies, which preserves + * the historical `error`-event behaviour exactly. + * + * One knob governs both terminal events. The cap is in BYTES, not + * characters: a character count is meaningless for budgeting a payload the + * moment stderr contains non-ASCII. + * + * @public + */ + stderrTailBytes?: number | null; /** Replaces the real resolveBinaryPath() call. */ _binaryResolver?: () => string; /** diff --git a/wrappers/typescript/dist/index.js b/wrappers/typescript/dist/index.js index d7319689..06e33928 100644 --- a/wrappers/typescript/dist/index.js +++ b/wrappers/typescript/dist/index.js @@ -39,7 +39,7 @@ export { Transport, parseNdjsonStream } from "./transport.js"; /** @public */ export { checkProtocolVersion } from "./version.js"; /** @public */ -export { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; +export { parseRunOutput, tailStderrBytes, STDERR_TAIL_BYTES, } from "./run-output-parser.js"; /** @public */ export { makeApprovalHandler } from "./approval.js"; // Internal imports used by spawnAgent(). @@ -52,8 +52,12 @@ import { checkProtocolVersion } from "./version.js"; * checked at `spawnAgent()` time against the engine's reported protocol * version (see Issue #9 โ€” `checkProtocolVersion()` is wired into the init * path so skew fails fast wrapper-side before any subprocess spawn). + * + * 0.4.0 added the envelope's per-turn usage block (`cacheReadTokens`, + * `cacheWriteTokens`, `costUsd`, and real `tokensIn`/`tokensOut`) that the + * `usage` field on the terminal `result` / `error` events surfaces. */ -export const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.3.0"; +export const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.4.0"; // --------------------------------------------------------------------------- // spawnAgent() โ€” locked public entry point (Mode A v2) // --------------------------------------------------------------------------- @@ -201,6 +205,13 @@ export async function spawnAgent(params) { // Issue #4: thread display.onEvent through so SessionHandle can // dispatch parsed NDJSON wire events to it. ...(params.display !== undefined ? { display: params.display } : {}), + // Forward the stderr-tail byte cap so the terminal result/error events + // carry exactly as much stderr as the host asked for. `null` is a + // meaningful value (the whole buffer), so it must be forwarded rather + // than treated as "unset". + ...(params.stderrTailBytes !== undefined + ? { stderrTailBytes: params.stderrTailBytes } + : {}), // Issue #7: persist engine metadata resolved during the probe. engineVersion: engineVersionPayload.version, bundleDigest: engineVersionPayload.bundleDigest ?? "", diff --git a/wrappers/typescript/dist/run-output-parser.d.ts b/wrappers/typescript/dist/run-output-parser.d.ts index 66ca4ccb..6b2b7ae9 100644 --- a/wrappers/typescript/dist/run-output-parser.d.ts +++ b/wrappers/typescript/dist/run-output-parser.d.ts @@ -11,13 +11,22 @@ * Rule 2 โ€” envelope absent / unparseable / partial โ†’ synthesize an error * event from exit code and stderr tail. Partial JSON is NOT * half-parsed (belt-and-suspenders): if any required ยง4.1 field - * is missing, the envelope is treated as unparseable. + * is missing, the envelope is treated as unparseable. The turn id + * is unknowable here (the engine assigns it), but the SESSION id is + * the caller's own โ€” pass it as `fallbackSessionId` and it is + * reported rather than dropped. * - * stderrTail is truncated to STDERR_TAIL_BYTES (4096) on synthesized paths; - * on the envelope path it is taken verbatim from the engine. + * On the Rule 1 path the envelope is now *read*, not merely shape-checked: + * `sessionId`, `turnId` and the `metadata` usage block are surfaced on the + * terminal event. The wrapper performs no arithmetic of its own โ€” the engine's + * UsageAccumulator already summed the turn, so re-summing here would + * double-count. + * + * `stderrTail` is bounded by `stderrTailBytes` in real UTF-8 BYTES (see + * `tailStderrBytes`); `STDERR_TAIL_BYTES` (4096) is the default. */ import type { DisplayEvent } from "./session.js"; -/** Maximum stderrTail length retained on synthesized engine errors. */ +/** Default cap on `stderrTail`, in BYTES of UTF-8. */ export declare const STDERR_TAIL_BYTES = 4096; /** Outcome of running the `amplifier-agent run --output json` subprocess. */ export interface SubprocessOutcome { @@ -25,9 +34,58 @@ export interface SubprocessOutcome { stderr: string; exitCode: number; } +/** Options for `parseRunOutput`. */ +export interface ParseRunOutputOptions { + /** + * Byte cap for `stderrTail` on the returned event. + * + * - a positive number โ€” at most that many UTF-8 BYTES, taken from the end. + * - `null` โ€” the ENTIRE stderr buffer, uncapped. + * - `0` โ€” capture disabled; `stderrTail` is omitted. + * - `undefined` โ€” not supplied; `STDERR_TAIL_BYTES` (4096) applies. + * + * The cap applies to whatever ends up in `stderrTail`, including a tail the + * engine supplied inside the envelope, so `0` really does mean "do not give + * me stderr". + */ + stderrTailBytes?: number | null; + /** + * The caller's own session id, reported on the SYNTHESIZED (Rule 2) events + * only. Never overrides the envelope: on the Rule 1 path the envelope's + * `sessionId` is authoritative and this option is ignored. + * + * `undefined` (the default) preserves the previous behaviour of omitting the + * field โ€” a caller with no session id to offer, such as a host post-parsing + * a captured payload, is not made to invent one. + */ + fallbackSessionId?: string; +} +/** + * Return the last `limit` BYTES of `text`, never splitting a codepoint. + * + * The cap is expressed in bytes because that is what a host budgeting a log + * line or a payload actually cares about; a character count is meaningless for + * that purpose the moment stderr contains non-ASCII. JavaScript strings are + * UTF-16 code units, so `String.prototype.slice` cannot express this โ€” the + * work happens on a `Buffer`. + * + * `limit` semantics: a positive number caps to that many UTF-8 bytes, `null` + * means the whole buffer, `undefined` falls back to `STDERR_TAIL_BYTES`, and + * `0` (or negative) disables capture and returns `undefined`. An empty `text` + * always returns `undefined`: there is nothing to report. + * + * Boundary safety: after slicing the encoded buffer, leading UTF-8 + * continuation bytes (`0b10xxxxxx`) are dropped so the slice begins on a lead + * byte. The result therefore decodes cleanly โ€” no U+FFFD โ€” at the cost of up + * to 3 bytes fewer than `limit`. Returning slightly less than the cap is + * correct; returning a broken character is not. + * + * @public + */ +export declare function tailStderrBytes(text: string, limit?: number | null | undefined): string | undefined; /** * Parse a subprocess outcome into a single DisplayEvent. * * See module docstring for precedence rules. */ -export declare function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent; +export declare function parseRunOutput(outcome: SubprocessOutcome, options?: ParseRunOutputOptions): DisplayEvent; diff --git a/wrappers/typescript/dist/run-output-parser.js b/wrappers/typescript/dist/run-output-parser.js index 9541edc2..39dfb1ea 100644 --- a/wrappers/typescript/dist/run-output-parser.js +++ b/wrappers/typescript/dist/run-output-parser.js @@ -11,26 +11,65 @@ * Rule 2 โ€” envelope absent / unparseable / partial โ†’ synthesize an error * event from exit code and stderr tail. Partial JSON is NOT * half-parsed (belt-and-suspenders): if any required ยง4.1 field - * is missing, the envelope is treated as unparseable. + * is missing, the envelope is treated as unparseable. The turn id + * is unknowable here (the engine assigns it), but the SESSION id is + * the caller's own โ€” pass it as `fallbackSessionId` and it is + * reported rather than dropped. * - * stderrTail is truncated to STDERR_TAIL_BYTES (4096) on synthesized paths; - * on the envelope path it is taken verbatim from the engine. + * On the Rule 1 path the envelope is now *read*, not merely shape-checked: + * `sessionId`, `turnId` and the `metadata` usage block are surfaced on the + * terminal event. The wrapper performs no arithmetic of its own โ€” the engine's + * UsageAccumulator already summed the turn, so re-summing here would + * double-count. + * + * `stderrTail` is bounded by `stderrTailBytes` in real UTF-8 BYTES (see + * `tailStderrBytes`); `STDERR_TAIL_BYTES` (4096) is the default. */ -/** Maximum stderrTail length retained on synthesized engine errors. */ +/** Default cap on `stderrTail`, in BYTES of UTF-8. */ export const STDERR_TAIL_BYTES = 4096; /** Maximum stdout snippet included in `envelope_missing` messages. */ const STDOUT_PREVIEW_BYTES = 512; /** - * Keep the last `STDERR_TAIL_BYTES` chars of `stderr`. - * Returns `undefined` for an empty string so callers can omit the field - * cleanly when there is nothing to surface. + * Return the last `limit` BYTES of `text`, never splitting a codepoint. + * + * The cap is expressed in bytes because that is what a host budgeting a log + * line or a payload actually cares about; a character count is meaningless for + * that purpose the moment stderr contains non-ASCII. JavaScript strings are + * UTF-16 code units, so `String.prototype.slice` cannot express this โ€” the + * work happens on a `Buffer`. + * + * `limit` semantics: a positive number caps to that many UTF-8 bytes, `null` + * means the whole buffer, `undefined` falls back to `STDERR_TAIL_BYTES`, and + * `0` (or negative) disables capture and returns `undefined`. An empty `text` + * always returns `undefined`: there is nothing to report. + * + * Boundary safety: after slicing the encoded buffer, leading UTF-8 + * continuation bytes (`0b10xxxxxx`) are dropped so the slice begins on a lead + * byte. The result therefore decodes cleanly โ€” no U+FFFD โ€” at the cost of up + * to 3 bytes fewer than `limit`. Returning slightly less than the cap is + * correct; returning a broken character is not. + * + * @public */ -function tailStderr(stderr) { - if (!stderr) +export function tailStderrBytes(text, limit = STDERR_TAIL_BYTES) { + if (!text) + return undefined; + if (limit === null) + return text; + const cap = limit === undefined ? STDERR_TAIL_BYTES : limit; + if (cap <= 0) return undefined; - if (stderr.length <= STDERR_TAIL_BYTES) - return stderr; - return stderr.slice(stderr.length - STDERR_TAIL_BYTES); + const raw = Buffer.from(text, "utf-8"); + if (raw.length <= cap) + return text; + let start = raw.length - cap; + // A UTF-8 continuation byte matches 0b10xxxxxx; a lead byte never does. The + // source is a valid string, so at most 3 continuation bytes can precede the + // first lead byte in the window. + while (start < raw.length && (raw[start] & 0xc0) === 0x80) { + start += 1; + } + return raw.subarray(start).toString("utf-8"); } const VALID_CLASSIFICATIONS = new Set([ "transport", @@ -39,6 +78,58 @@ const VALID_CLASSIFICATIONS = new Set([ "approval", "unknown", ]); +/** + * The envelope `metadata` keys that make up a usage report. Presence of at + * least one of them is what distinguishes "the engine reported usage" from + * "this engine predates protocol 0.4.0 and reported none". + */ +const USAGE_KEYS = [ + "tokensIn", + "tokensOut", + "cacheReadTokens", + "cacheWriteTokens", + "costUsd", +]; +/** + * Coerce a wire token count to a number, defaulting to 0. + * + * Deliberately total: a malformed count must not be able to turn a completed + * turn into a throw on the host's side. + */ +function toInt(value) { + if (typeof value === "number" && Number.isFinite(value)) + return Math.trunc(value); + if (typeof value === "string") { + const parsed = Number(value); + if (Number.isFinite(parsed)) + return Math.trunc(parsed); + } + return 0; +} +/** + * Read the envelope's `metadata` usage block into a `Usage`. + * + * Returns `undefined` when the metadata carries none of the usage keys at all + * โ€” an engine older than protocol 0.4.0 never reported them, and claiming + * zeroes on its behalf would be a fabricated number rather than an absent one. + */ +function usageFromMetadata(metadata) { + if (!USAGE_KEYS.some((key) => key in metadata)) + return undefined; + // costUsd stays a STRING on the TS side. See the `Usage` doc comment in + // session.ts for why this is a deliberate parity exception. + const rawCost = metadata.costUsd; + const costUsd = typeof rawCost === "string" ? rawCost : rawCost == null ? null : String(rawCost); + return { + // Mirrors the envelope verbatim: tokensIn is the CHARGED total the engine + // already computed. No wrapper-side arithmetic. + inputTokens: toInt(metadata.tokensIn), + outputTokens: toInt(metadata.tokensOut), + cacheReadTokens: toInt(metadata.cacheReadTokens), + cacheWriteTokens: toInt(metadata.cacheWriteTokens), + costUsd, + }; +} /** * Validate that `parsed` conforms to the ยง4.1 envelope shape. * @@ -77,7 +168,9 @@ function isShapeValid(parsed) { * * See module docstring for precedence rules. */ -export function parseRunOutput(outcome) { +export function parseRunOutput(outcome, options = {}) { + const tailBytes = options.stderrTailBytes; + const fallbackSessionId = options.fallbackSessionId; const trimmed = outcome.stdout.trim(); // Attempt to parse stdout as JSON. Failures (empty, partial, non-JSON) are // captured silently; the caller falls to Rule 2. @@ -93,9 +186,21 @@ export function parseRunOutput(outcome) { // Rule 1 โ€” envelope parseable per ยง4.1 โ†’ envelope wins. if (parsed !== null && isShapeValid(parsed)) { const env = parsed; + const usage = usageFromMetadata(env.metadata); if (env.error === null) { - // Success path โ€” exit code is informational only. - return { type: "result", text: env.reply }; + // Success path โ€” exit code is informational only, but still reported as + // observed so a host can see a post-flush crash (a result event carrying + // a non-zero exitCode). + const stderrTail = tailStderrBytes(outcome.stderr, tailBytes); + return { + type: "result", + text: env.reply, + sessionId: env.sessionId, + turnId: env.turnId, + exitCode: outcome.exitCode, + ...(usage !== undefined ? { usage } : {}), + ...(stderrTail !== undefined ? { stderrTail } : {}), + }; } // Failure path โ€” populate from the envelope's error fields. const err = env.error; @@ -106,7 +211,8 @@ export function parseRunOutput(outcome) { const severity = err.severity === "warning" ? "warning" : "error"; const correlationId = typeof err.correlationId === "string" ? err.correlationId : ""; const message = typeof err.message === "string" ? err.message : err.code; - const stderrTail = typeof err.stderrTail === "string" ? err.stderrTail : tailStderr(outcome.stderr); + const sourceTail = typeof err.stderrTail === "string" ? err.stderrTail : outcome.stderr; + const stderrTail = tailStderrBytes(sourceTail, tailBytes); return { type: "error", code: err.code, @@ -116,10 +222,21 @@ export function parseRunOutput(outcome) { message, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + sessionId: env.sessionId, + turnId: env.turnId, + exitCode: outcome.exitCode, + ...(usage !== undefined ? { usage } : {}), }; } // Rule 2 โ€” envelope absent or unparseable โ†’ synthesize from exit + stderr. - const stderrTail = tailStderr(outcome.stderr); + // No envelope means no turnId and no usage to report: the engine assigns the + // turn id and nothing came back, so inventing one would be a fabrication. + // The SESSION id is different โ€” the caller minted it and passed it in as + // `fallbackSessionId`, so a host correlating this failure against its own + // records still gets the handle it already knows. The exit code remains + // load-bearing for the code/classification split below. + const stderrTail = tailStderrBytes(outcome.stderr, tailBytes); + const sessionIdField = fallbackSessionId !== undefined ? { sessionId: fallbackSessionId } : {}; if (outcome.exitCode === 0) { // Engine protocol violation: exit 0 without a parseable envelope. const preview = outcome.stdout.slice(0, STDOUT_PREVIEW_BYTES); @@ -133,6 +250,8 @@ export function parseRunOutput(outcome) { message: `Engine exited 0 without emitting a parseable ยง4.1 envelope. Stdout was: ${JSON.stringify(preview)}${previewSuffix}`, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + ...sessionIdField, + exitCode: outcome.exitCode, }; } // Non-zero exit, envelope absent or partial โ€” engine-class failure. @@ -145,5 +264,7 @@ export function parseRunOutput(outcome) { message: `Engine exited ${outcome.exitCode} without emitting a parseable ยง4.1 envelope.`, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + ...sessionIdField, + exitCode: outcome.exitCode, }; } diff --git a/wrappers/typescript/dist/session.d.ts b/wrappers/typescript/dist/session.d.ts index 69224ff2..5816606f 100644 --- a/wrappers/typescript/dist/session.d.ts +++ b/wrappers/typescript/dist/session.d.ts @@ -40,23 +40,110 @@ import type { McpServerConfig } from "./types.js"; * @public */ export type ChildProcessFactory = (command: string, args: readonly string[], options: SpawnOptions) => ChildProcess; +/** + * Per-turn token and cost accounting, as reported by the engine. + * + * Read straight off the ยง4.1 envelope's `metadata` block. The wrapper does NOT + * sum anything: the engine's `UsageAccumulator` already folded every `usage` + * display event of the turn (including sub-agent LLM calls) into these totals, + * so re-summing wrapper-side would double-count. + * + * PARITY NOTE โ€” `costUsd` is the one field where the two wrappers deliberately + * differ. The Python wrapper parses it into a `Decimal`; TypeScript keeps the + * decimal STRING the engine put on the wire. JavaScript has no decimal type, + * and `number` is an IEEE-754 binary double: `0.1 + 0.2 !== 0.3`, so parsing a + * monetary value into one loses precision the moment a host sums a few turns. + * Handing back the exact string the engine emitted lets a host feed it to + * whichever decimal library it already uses. This asymmetry is a language + * constraint, not drift โ€” see `Usage.cost_usd` in + * wrappers/python-py/src/amplifier_agent_py/types.py. + * + * @public + */ +export interface Usage { + /** + * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is new + * input + cache reads + cache writes; the model saw all three as input and + * the split is a billing distinction. A host that wants the new-only figure + * derives it as `inputTokens - cacheReadTokens - cacheWriteTokens`. + */ + inputTokens: number; + /** Output tokens (envelope `tokensOut`). */ + outputTokens: number; + /** Input tokens served from the provider's prompt cache. */ + cacheReadTokens: number; + /** Input tokens written into the provider's prompt cache. */ + cacheWriteTokens: number; + /** + * Turn cost as a decimal STRING (never a number โ€” see the parity note on + * this interface). `null` when no provider reported a cost; that is not the + * same claim as `"0"`. + */ + costUsd: string | null; +} /** * A display event yielded by `SessionHandle.submit()`. * * Mode A v2 (CR-C, amendment ยง5.2): a discriminated union narrow enough that * every variant's payload is exhaustively typed. The fields removed from the - * pre-amendment shape (`turnId`, `parentTurnId`, `synthesized`, `payload`) - * cannot be meaningfully populated on the Mode A wire. + * pre-amendment shape (`parentTurnId`, `synthesized`, `payload`) cannot be + * meaningfully populated on the Mode A wire. */ export type DisplayEvent = { type: "init"; sessionId: string; } | { type: "activity"; -} | { +} +/** + * Terminal success event. `sessionId` / `turnId` / `exitCode` are never + * optional: a `result` event exists only on the envelope path, so the + * identity fields the envelope carries are always known. (The `error` + * variant can be synthesized with no envelope at all, which is why its + * equivalents are optional there.) + * + * `usage` is absent only when the envelope's `metadata` carried no usage + * keys whatsoever โ€” an engine older than protocol 0.4.0. Absent means "not + * reported", which is a different claim from a present `Usage` reading zero + * (a turn that made no LLM call really did spend nothing). + * + * `stderrTail` holds the last `stderrTailBytes` BYTES of the engine's + * stderr, or the whole buffer when that option is `null`, or is omitted when + * it is `0` or stderr was empty. + */ + | { type: "result"; text: string; -} | { + sessionId: string; + turnId: string; + exitCode: number; + usage?: Usage; + stderrTail?: string; +} +/** + * Terminal failure event. `sessionId` / `turnId` / `exitCode` / `usage` are + * populated from the ยง4.1 envelope when one was parsed. + * + * On the synthesized (Rule 2) paths โ€” envelope absent, unparseable, spawn + * failure, or hang โ€” there is no envelope to read them from, and the fields + * split by who knows the answer: + * + * - `sessionId` IS present whenever the wrapper itself knows it, which is + * every failure raised through a `SessionHandle`: the handle was given the + * session id at construction time, so a host correlating the failure gets + * the same identifier it passed in. It is omitted only when + * `parseRunOutput` is called directly without a `fallbackSessionId`. + * - `turnId` is omitted. The engine assigns turn ids and no envelope came + * back, so the wrapper genuinely does not know it and will not invent one. + * - `exitCode` is present on the parser's Rule 2 paths (the process did + * exit) and omitted on spawn failure and hang, where it never did. + * - `usage` is omitted: only the envelope reports it. + * + * `usage` on the failure path is not a duplicate report: nothing else on the + * failure path carries usage, so a turn that burned tokens and then failed + * would otherwise spend them invisibly. + */ + | { type: "error"; code: string; classification: "transport" | "protocol" | "engine" | "approval" | "unknown"; @@ -65,6 +152,10 @@ export type DisplayEvent = { message: string; stderrTail?: string; retryable: boolean; + sessionId?: string; + turnId?: string; + exitCode?: number; + usage?: Usage; } /** * Wire-protocol notification dispatched from the engine's stderr NDJSON @@ -179,8 +270,20 @@ export interface SessionHandleParams { * invalid slugs with `argv_workspace_invalid`. */ workspace?: string; - /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ + /** Protocol version the wrapper speaks (e.g. "0.4.0"). */ protocolVersion: string; + /** + * Byte cap on `stderrTail` for BOTH the terminal `result` and `error` + * events. + * + * - a positive number โ€” the last N UTF-8 BYTES, never split mid-codepoint. + * - `null` โ€” the ENTIRE stderr buffer. + * - `0` โ€” capture disabled; the field is omitted. + * - `undefined` โ€” not supplied; `STDERR_TAIL_BYTES` (4096) applies, + * which preserves the historical `error`-event + * behaviour exactly. + */ + stderrTailBytes?: number | null; /** * Per-submit timeout in milliseconds. No timeout is applied unless a * positive value is provided. `undefined` or `<= 0` disables the diff --git a/wrappers/typescript/dist/session.js b/wrappers/typescript/dist/session.js index 6a9f545e..7526e738 100644 --- a/wrappers/typescript/dist/session.js +++ b/wrappers/typescript/dist/session.js @@ -28,7 +28,7 @@ import { spawn as childSpawn } from "node:child_process"; import { assembleArgv } from "./argv-builder.js"; import { resolveMcpConfigPath, cleanupSpillFile } from "./mcp-spill.js"; import { resolvePromptFilePath, cleanupPromptSpillFile, } from "./prompt-spill.js"; -import { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; +import { parseRunOutput, tailStderrBytes } from "./run-output-parser.js"; import { parseNdjsonStream } from "./transport.js"; /** Typed error for AaA wrapper lifecycle and protocol violations. */ export class AaaError extends Error { @@ -86,14 +86,6 @@ export function waitForExitOrTimeout(child, ms) { }); }); } -/** Last `STDERR_TAIL_BYTES` chars of `stderr`, or undefined if empty. */ -function stderrTailOf(stderr) { - if (!stderr) - return undefined; - if (stderr.length <= STDERR_TAIL_BYTES) - return stderr; - return stderr.slice(stderr.length - STDERR_TAIL_BYTES); -} /** One-shot session handle that drives the engine subprocess. */ export class SessionHandle { params; @@ -288,7 +280,7 @@ export class SessionHandle { timeoutHandle = setTimeout(() => { // Synthesize engine_hung before invoking cancel(), so the iterator // yields a terminal error even if SIGTERM/SIGKILL hangs. - const tail = stderrTailOf(stderrBuf); + const tail = tailStderrBytes(stderrBuf, this.params.stderrTailBytes); finalize({ type: "error", code: "engine_hung", @@ -298,6 +290,9 @@ export class SessionHandle { message: `Engine subprocess hung past ${timeoutMs}ms timeout; SIGTERM/SIGKILL escalation invoked.`, ...(tail !== undefined ? { stderrTail: tail } : {}), retryable: false, + // The engine never reported a turn id, but the session id is ours โ€” + // report it so a hung turn is still correlatable. + sessionId: this.params.sessionId, }); // Fire-and-forget: cancel races the next event-loop turn. void this.cancel(); @@ -312,6 +307,13 @@ export class SessionHandle { stdout: stdoutBuf, stderr: stderrBuf, exitCode: code ?? -1, + }, { + stderrTailBytes: this.params.stderrTailBytes, + // Rule 2 synthesis has no envelope to read identity from; hand the + // parser the session id we already hold so the synthesized error + // still carries it. Ignored on the Rule 1 path, where the envelope + // is authoritative. + fallbackSessionId: this.params.sessionId, }); finalize(ev); }); @@ -323,7 +325,7 @@ export class SessionHandle { clearTimeout(timeoutHandle); if (finalized) return; - const tail = stderrTailOf(stderrBuf); + const tail = tailStderrBytes(stderrBuf, this.params.stderrTailBytes); finalize({ type: "error", code: "spawn_failed", @@ -333,6 +335,9 @@ export class SessionHandle { message: `Failed to spawn engine subprocess (${err.code ?? "unknown"}): ${err.message}`, ...(tail !== undefined ? { stderrTail: tail } : {}), retryable: false, + // No engine ran, so there is no turn id to report โ€” but the session id + // is the handle's own, and a host correlating this failure needs it. + sessionId: this.params.sessionId, }); }); // (ix) drain loop โ€” yield activity events then the final event. diff --git a/wrappers/typescript/dist/types.d.ts b/wrappers/typescript/dist/types.d.ts index 18310f43..ecb9b657 100644 --- a/wrappers/typescript/dist/types.d.ts +++ b/wrappers/typescript/dist/types.d.ts @@ -275,6 +275,11 @@ export interface TurnSubmitResult { reply: string | null; turnId: string; sessionId: string; + tokensIn: number; + tokensOut: number; + cacheReadTokens: number; + cacheWriteTokens: number; + costUsd: string | null; finalEvent?: { [k: string]: unknown; }; diff --git a/wrappers/typescript/src/argv-builder.ts b/wrappers/typescript/src/argv-builder.ts index 396c9e24..5a66414b 100644 --- a/wrappers/typescript/src/argv-builder.ts +++ b/wrappers/typescript/src/argv-builder.ts @@ -28,7 +28,7 @@ export interface AssembleArgvInput { * Defaults to undefined, so callers that never spill are unaffected. */ promptFile?: string; - /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ + /** Protocol version the wrapper speaks (e.g. "0.4.0"). */ protocolVersion: string; /** When true, emit `--resume` instead of `--fresh`. */ resume?: boolean; diff --git a/wrappers/typescript/src/index.ts b/wrappers/typescript/src/index.ts index 8ae63fe4..7437938e 100644 --- a/wrappers/typescript/src/index.ts +++ b/wrappers/typescript/src/index.ts @@ -14,6 +14,7 @@ export type { DisplayEvent, EngineInfo, SessionHandleParams, + Usage, } from "./session.js"; export type { ApprovalResponse } from "./approval.js"; export type { EngineVersionPayload } from "./spawn.js"; @@ -92,9 +93,16 @@ export type { } from "./version.js"; /** @public */ -export { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; +export { + parseRunOutput, + tailStderrBytes, + STDERR_TAIL_BYTES, +} from "./run-output-parser.js"; /** @public */ -export type { SubprocessOutcome } from "./run-output-parser.js"; +export type { + SubprocessOutcome, + ParseRunOutputOptions, +} from "./run-output-parser.js"; /** @public */ export { makeApprovalHandler } from "./approval.js"; @@ -129,8 +137,12 @@ export type { ChildProcessFactory } from "./session.js"; * checked at `spawnAgent()` time against the engine's reported protocol * version (see Issue #9 โ€” `checkProtocolVersion()` is wired into the init * path so skew fails fast wrapper-side before any subprocess spawn). + * + * 0.4.0 added the envelope's per-turn usage block (`cacheReadTokens`, + * `cacheWriteTokens`, `costUsd`, and real `tokensIn`/`tokensOut`) that the + * `usage` field on the terminal `result` / `error` events surfaces. */ -export const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.3.0"; +export const PROTOCOL_VERSION_REQUIRED_BY_WRAPPER = "0.4.0"; // --------------------------------------------------------------------------- // SpawnAgentParams โ€” locked public API (design ยง8.2, amended for Mode A v2) @@ -257,6 +269,24 @@ export interface SpawnAgentParams { * Mirrors the engine-side `host_config.allowProtocolSkew` knob. */ allowProtocolSkew?: boolean; + /** + * Byte cap on `stderrTail` for the terminal `result` / `error` events. + * + * - a positive number โ€” the last N UTF-8 BYTES of the engine's stderr, + * never split mid-codepoint. Trimming to a codepoint boundary can yield + * slightly fewer than N bytes; that is correct. + * - `null` โ€” the ENTIRE stderr buffer, uncapped. + * - `0` โ€” capture disabled; `stderrTail` is omitted. + * - omitted โ€” `STDERR_TAIL_BYTES` (4096) applies, which preserves + * the historical `error`-event behaviour exactly. + * + * One knob governs both terminal events. The cap is in BYTES, not + * characters: a character count is meaningless for budgeting a payload the + * moment stderr contains non-ASCII. + * + * @public + */ + stderrTailBytes?: number | null; // ------------------------------------------------------------------ // Test-only injection points (undocumented in public API). @@ -452,6 +482,13 @@ export async function spawnAgent(params: SpawnAgentParams): Promise = new Set([ "unknown", ]); +/** + * The envelope `metadata` keys that make up a usage report. Presence of at + * least one of them is what distinguishes "the engine reported usage" from + * "this engine predates protocol 0.4.0 and reported none". + */ +const USAGE_KEYS = [ + "tokensIn", + "tokensOut", + "cacheReadTokens", + "cacheWriteTokens", + "costUsd", +] as const; + +/** + * Coerce a wire token count to a number, defaulting to 0. + * + * Deliberately total: a malformed count must not be able to turn a completed + * turn into a throw on the host's side. + */ +function toInt(value: unknown): number { + if (typeof value === "number" && Number.isFinite(value)) return Math.trunc(value); + if (typeof value === "string") { + const parsed = Number(value); + if (Number.isFinite(parsed)) return Math.trunc(parsed); + } + return 0; +} + +/** + * Read the envelope's `metadata` usage block into a `Usage`. + * + * Returns `undefined` when the metadata carries none of the usage keys at all + * โ€” an engine older than protocol 0.4.0 never reported them, and claiming + * zeroes on its behalf would be a fabricated number rather than an absent one. + */ +function usageFromMetadata(metadata: Record): Usage | undefined { + if (!USAGE_KEYS.some((key) => key in metadata)) return undefined; + + // costUsd stays a STRING on the TS side. See the `Usage` doc comment in + // session.ts for why this is a deliberate parity exception. + const rawCost = metadata.costUsd; + const costUsd = typeof rawCost === "string" ? rawCost : rawCost == null ? null : String(rawCost); + + return { + // Mirrors the envelope verbatim: tokensIn is the CHARGED total the engine + // already computed. No wrapper-side arithmetic. + inputTokens: toInt(metadata.tokensIn), + outputTokens: toInt(metadata.tokensOut), + cacheReadTokens: toInt(metadata.cacheReadTokens), + cacheWriteTokens: toInt(metadata.cacheWriteTokens), + costUsd, + }; +} + /** * Validate that `parsed` conforms to the ยง4.1 envelope shape. * @@ -98,7 +221,12 @@ function isShapeValid(parsed: unknown): parsed is { * * See module docstring for precedence rules. */ -export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { +export function parseRunOutput( + outcome: SubprocessOutcome, + options: ParseRunOutputOptions = {}, +): DisplayEvent { + const tailBytes = options.stderrTailBytes; + const fallbackSessionId = options.fallbackSessionId; const trimmed = outcome.stdout.trim(); // Attempt to parse stdout as JSON. Failures (empty, partial, non-JSON) are @@ -115,10 +243,22 @@ export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { // Rule 1 โ€” envelope parseable per ยง4.1 โ†’ envelope wins. if (parsed !== null && isShapeValid(parsed)) { const env = parsed; + const usage = usageFromMetadata(env.metadata); if (env.error === null) { - // Success path โ€” exit code is informational only. - return { type: "result", text: env.reply }; + // Success path โ€” exit code is informational only, but still reported as + // observed so a host can see a post-flush crash (a result event carrying + // a non-zero exitCode). + const stderrTail = tailStderrBytes(outcome.stderr, tailBytes); + return { + type: "result", + text: env.reply, + sessionId: env.sessionId, + turnId: env.turnId, + exitCode: outcome.exitCode, + ...(usage !== undefined ? { usage } : {}), + ...(stderrTail !== undefined ? { stderrTail } : {}), + }; } // Failure path โ€” populate from the envelope's error fields. @@ -133,8 +273,9 @@ export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { const correlationId = typeof err.correlationId === "string" ? err.correlationId : ""; const message = typeof err.message === "string" ? err.message : err.code; - const stderrTail = - typeof err.stderrTail === "string" ? err.stderrTail : tailStderr(outcome.stderr); + const sourceTail = + typeof err.stderrTail === "string" ? err.stderrTail : outcome.stderr; + const stderrTail = tailStderrBytes(sourceTail, tailBytes); return { type: "error", @@ -145,11 +286,23 @@ export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { message, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + sessionId: env.sessionId, + turnId: env.turnId, + exitCode: outcome.exitCode, + ...(usage !== undefined ? { usage } : {}), }; } // Rule 2 โ€” envelope absent or unparseable โ†’ synthesize from exit + stderr. - const stderrTail = tailStderr(outcome.stderr); + // No envelope means no turnId and no usage to report: the engine assigns the + // turn id and nothing came back, so inventing one would be a fabrication. + // The SESSION id is different โ€” the caller minted it and passed it in as + // `fallbackSessionId`, so a host correlating this failure against its own + // records still gets the handle it already knows. The exit code remains + // load-bearing for the code/classification split below. + const stderrTail = tailStderrBytes(outcome.stderr, tailBytes); + const sessionIdField = + fallbackSessionId !== undefined ? { sessionId: fallbackSessionId } : {}; if (outcome.exitCode === 0) { // Engine protocol violation: exit 0 without a parseable envelope. @@ -165,6 +318,8 @@ export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { message: `Engine exited 0 without emitting a parseable ยง4.1 envelope. Stdout was: ${JSON.stringify(preview)}${previewSuffix}`, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + ...sessionIdField, + exitCode: outcome.exitCode, }; } @@ -178,5 +333,7 @@ export function parseRunOutput(outcome: SubprocessOutcome): DisplayEvent { message: `Engine exited ${outcome.exitCode} without emitting a parseable ยง4.1 envelope.`, ...(stderrTail !== undefined ? { stderrTail } : {}), retryable: false, + ...sessionIdField, + exitCode: outcome.exitCode, }; } diff --git a/wrappers/typescript/src/session.ts b/wrappers/typescript/src/session.ts index 16dc3648..436a2356 100644 --- a/wrappers/typescript/src/session.ts +++ b/wrappers/typescript/src/session.ts @@ -34,7 +34,7 @@ import { resolvePromptFilePath, cleanupPromptSpillFile, } from "./prompt-spill.js"; -import { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; +import { parseRunOutput, tailStderrBytes } from "./run-output-parser.js"; import { parseNdjsonStream } from "./transport.js"; import type { McpServerConfig } from "./types.js"; @@ -57,18 +57,107 @@ export type ChildProcessFactory = ( options: SpawnOptions, ) => ChildProcess; +/** + * Per-turn token and cost accounting, as reported by the engine. + * + * Read straight off the ยง4.1 envelope's `metadata` block. The wrapper does NOT + * sum anything: the engine's `UsageAccumulator` already folded every `usage` + * display event of the turn (including sub-agent LLM calls) into these totals, + * so re-summing wrapper-side would double-count. + * + * PARITY NOTE โ€” `costUsd` is the one field where the two wrappers deliberately + * differ. The Python wrapper parses it into a `Decimal`; TypeScript keeps the + * decimal STRING the engine put on the wire. JavaScript has no decimal type, + * and `number` is an IEEE-754 binary double: `0.1 + 0.2 !== 0.3`, so parsing a + * monetary value into one loses precision the moment a host sums a few turns. + * Handing back the exact string the engine emitted lets a host feed it to + * whichever decimal library it already uses. This asymmetry is a language + * constraint, not drift โ€” see `Usage.cost_usd` in + * wrappers/python-py/src/amplifier_agent_py/types.py. + * + * @public + */ +export interface Usage { + /** + * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is new + * input + cache reads + cache writes; the model saw all three as input and + * the split is a billing distinction. A host that wants the new-only figure + * derives it as `inputTokens - cacheReadTokens - cacheWriteTokens`. + */ + inputTokens: number; + /** Output tokens (envelope `tokensOut`). */ + outputTokens: number; + /** Input tokens served from the provider's prompt cache. */ + cacheReadTokens: number; + /** Input tokens written into the provider's prompt cache. */ + cacheWriteTokens: number; + /** + * Turn cost as a decimal STRING (never a number โ€” see the parity note on + * this interface). `null` when no provider reported a cost; that is not the + * same claim as `"0"`. + */ + costUsd: string | null; +} + /** * A display event yielded by `SessionHandle.submit()`. * * Mode A v2 (CR-C, amendment ยง5.2): a discriminated union narrow enough that * every variant's payload is exhaustively typed. The fields removed from the - * pre-amendment shape (`turnId`, `parentTurnId`, `synthesized`, `payload`) - * cannot be meaningfully populated on the Mode A wire. + * pre-amendment shape (`parentTurnId`, `synthesized`, `payload`) cannot be + * meaningfully populated on the Mode A wire. */ export type DisplayEvent = | { type: "init"; sessionId: string } | { type: "activity" } - | { type: "result"; text: string } + /** + * Terminal success event. `sessionId` / `turnId` / `exitCode` are never + * optional: a `result` event exists only on the envelope path, so the + * identity fields the envelope carries are always known. (The `error` + * variant can be synthesized with no envelope at all, which is why its + * equivalents are optional there.) + * + * `usage` is absent only when the envelope's `metadata` carried no usage + * keys whatsoever โ€” an engine older than protocol 0.4.0. Absent means "not + * reported", which is a different claim from a present `Usage` reading zero + * (a turn that made no LLM call really did spend nothing). + * + * `stderrTail` holds the last `stderrTailBytes` BYTES of the engine's + * stderr, or the whole buffer when that option is `null`, or is omitted when + * it is `0` or stderr was empty. + */ + | { + type: "result"; + text: string; + sessionId: string; + turnId: string; + exitCode: number; + usage?: Usage; + stderrTail?: string; + } + /** + * Terminal failure event. `sessionId` / `turnId` / `exitCode` / `usage` are + * populated from the ยง4.1 envelope when one was parsed. + * + * On the synthesized (Rule 2) paths โ€” envelope absent, unparseable, spawn + * failure, or hang โ€” there is no envelope to read them from, and the fields + * split by who knows the answer: + * + * - `sessionId` IS present whenever the wrapper itself knows it, which is + * every failure raised through a `SessionHandle`: the handle was given the + * session id at construction time, so a host correlating the failure gets + * the same identifier it passed in. It is omitted only when + * `parseRunOutput` is called directly without a `fallbackSessionId`. + * - `turnId` is omitted. The engine assigns turn ids and no envelope came + * back, so the wrapper genuinely does not know it and will not invent one. + * - `exitCode` is present on the parser's Rule 2 paths (the process did + * exit) and omitted on spawn failure and hang, where it never did. + * - `usage` is omitted: only the envelope reports it. + * + * `usage` on the failure path is not a duplicate report: nothing else on the + * failure path carries usage, so a turn that burned tokens and then failed + * would otherwise spend them invisibly. + */ | { type: "error"; code: string; @@ -78,6 +167,10 @@ export type DisplayEvent = message: string; stderrTail?: string; retryable: boolean; + sessionId?: string; + turnId?: string; + exitCode?: number; + usage?: Usage; } /** * Wire-protocol notification dispatched from the engine's stderr NDJSON @@ -207,8 +300,20 @@ export interface SessionHandleParams { * invalid slugs with `argv_workspace_invalid`. */ workspace?: string; - /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ + /** Protocol version the wrapper speaks (e.g. "0.4.0"). */ protocolVersion: string; + /** + * Byte cap on `stderrTail` for BOTH the terminal `result` and `error` + * events. + * + * - a positive number โ€” the last N UTF-8 BYTES, never split mid-codepoint. + * - `null` โ€” the ENTIRE stderr buffer. + * - `0` โ€” capture disabled; the field is omitted. + * - `undefined` โ€” not supplied; `STDERR_TAIL_BYTES` (4096) applies, + * which preserves the historical `error`-event + * behaviour exactly. + */ + stderrTailBytes?: number | null; /** * Per-submit timeout in milliseconds. No timeout is applied unless a * positive value is provided. `undefined` or `<= 0` disables the @@ -275,13 +380,6 @@ export function waitForExitOrTimeout(child: ChildProcess, ms: number): Promise { // Synthesize engine_hung before invoking cancel(), so the iterator // yields a terminal error even if SIGTERM/SIGKILL hangs. - const tail = stderrTailOf(stderrBuf); + const tail = tailStderrBytes(stderrBuf, this.params.stderrTailBytes); finalize({ type: "error", code: "engine_hung", @@ -511,6 +609,9 @@ export class SessionHandle { message: `Engine subprocess hung past ${timeoutMs}ms timeout; SIGTERM/SIGKILL escalation invoked.`, ...(tail !== undefined ? { stderrTail: tail } : {}), retryable: false, + // The engine never reported a turn id, but the session id is ours โ€” + // report it so a hung turn is still correlatable. + sessionId: this.params.sessionId, }); // Fire-and-forget: cancel races the next event-loop turn. void this.cancel(); @@ -520,11 +621,21 @@ export class SessionHandle { child.once("exit", (code: number | null, _signal: NodeJS.Signals | null) => { if (timeoutHandle !== null) clearTimeout(timeoutHandle); if (finalized) return; - const ev = parseRunOutput({ - stdout: stdoutBuf, - stderr: stderrBuf, - exitCode: code ?? -1, - }); + const ev = parseRunOutput( + { + stdout: stdoutBuf, + stderr: stderrBuf, + exitCode: code ?? -1, + }, + { + stderrTailBytes: this.params.stderrTailBytes, + // Rule 2 synthesis has no envelope to read identity from; hand the + // parser the session id we already hold so the synthesized error + // still carries it. Ignored on the Rule 1 path, where the envelope + // is authoritative. + fallbackSessionId: this.params.sessionId, + }, + ); finalize(ev); }); @@ -534,7 +645,7 @@ export class SessionHandle { child.once("error", (err: NodeJS.ErrnoException) => { if (timeoutHandle !== null) clearTimeout(timeoutHandle); if (finalized) return; - const tail = stderrTailOf(stderrBuf); + const tail = tailStderrBytes(stderrBuf, this.params.stderrTailBytes); finalize({ type: "error", code: "spawn_failed", @@ -544,6 +655,9 @@ export class SessionHandle { message: `Failed to spawn engine subprocess (${err.code ?? "unknown"}): ${err.message}`, ...(tail !== undefined ? { stderrTail: tail } : {}), retryable: false, + // No engine ran, so there is no turn id to report โ€” but the session id + // is the handle's own, and a host correlating this failure needs it. + sessionId: this.params.sessionId, }); }); diff --git a/wrappers/typescript/src/types.ts b/wrappers/typescript/src/types.ts index 759df6c2..f8e7cdc0 100644 --- a/wrappers/typescript/src/types.ts +++ b/wrappers/typescript/src/types.ts @@ -306,6 +306,11 @@ export interface TurnSubmitResult { reply: string | null; turnId: string; sessionId: string; + tokensIn: number; + tokensOut: number; + cacheReadTokens: number; + cacheWriteTokens: number; + costUsd: string | null; finalEvent?: { [k: string]: unknown; }; diff --git a/wrappers/typescript/test/argv-builder.test.ts b/wrappers/typescript/test/argv-builder.test.ts index 5dcfbab6..88691a8a 100644 --- a/wrappers/typescript/test/argv-builder.test.ts +++ b/wrappers/typescript/test/argv-builder.test.ts @@ -157,7 +157,7 @@ describe("assembleArgv", () => { const input: AssembleArgvInput = { sessionId: "sid", prompt: "hello", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", // @ts-expect-error -- providerOverride was removed from AssembleArgvInput. providerOverride: "anthropic", }; @@ -173,7 +173,7 @@ describe("assembleArgv", () => { const input: AssembleArgvInput = { sessionId: "sid", prompt: "hello", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", // @ts-expect-error -- modelOverride was removed from AssembleArgvInput. modelOverride: "claude-sonnet-4-5", }; @@ -189,7 +189,7 @@ describe("assembleArgv", () => { const input: AssembleArgvInput = { sessionId: "sid", prompt: "hello", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", // @ts-expect-error -- effortOverride was removed from AssembleArgvInput. effortOverride: "high", }; diff --git a/wrappers/typescript/test/issue-1-configpath.test.ts b/wrappers/typescript/test/issue-1-configpath.test.ts index 1c890f46..7a251986 100644 --- a/wrappers/typescript/test/issue-1-configpath.test.ts +++ b/wrappers/typescript/test/issue-1-configpath.test.ts @@ -28,7 +28,7 @@ describe("Issue #1 โ€” SpawnAgentParams.configPath threads to --config argv", () const argv = assembleArgv({ sessionId: "s1", prompt: "hello", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", configPath: "/etc/amplifier/host_config.json", }); const idx = argv.indexOf("--config"); @@ -40,7 +40,7 @@ describe("Issue #1 โ€” SpawnAgentParams.configPath threads to --config argv", () const argv = assembleArgv({ sessionId: "s1", prompt: "hello", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", }); expect(argv).not.toContain("--config"); }); diff --git a/wrappers/typescript/test/issue-10-approval.test.ts b/wrappers/typescript/test/issue-10-approval.test.ts index a0b7d044..a5e9c523 100644 --- a/wrappers/typescript/test/issue-10-approval.test.ts +++ b/wrappers/typescript/test/issue-10-approval.test.ts @@ -98,7 +98,7 @@ describe("Issue #10 โ€” approval API maps to engine -y/-n argv", () => { const argv = assembleArgv({ sessionId: "s", prompt: "p", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", approvalMode: "yes", }); expect(argv).toContain("-y"); @@ -109,7 +109,7 @@ describe("Issue #10 โ€” approval API maps to engine -y/-n argv", () => { const argv = assembleArgv({ sessionId: "s", prompt: "p", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", approvalMode: "no", }); expect(argv).toContain("-n"); @@ -120,7 +120,7 @@ describe("Issue #10 โ€” approval API maps to engine -y/-n argv", () => { const argv = assembleArgv({ sessionId: "s", prompt: "p", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", approvalMode: "prompt", }); expect(argv).not.toContain("-y"); @@ -131,7 +131,7 @@ describe("Issue #10 โ€” approval API maps to engine -y/-n argv", () => { const argv = assembleArgv({ sessionId: "s", prompt: "p", - protocolVersion: "0.3.0", + protocolVersion: "0.4.0", }); expect(argv).toContain("-y"); }); diff --git a/wrappers/typescript/test/run-output-parser.test.ts b/wrappers/typescript/test/run-output-parser.test.ts index b6577296..04c000ba 100644 --- a/wrappers/typescript/test/run-output-parser.test.ts +++ b/wrappers/typescript/test/run-output-parser.test.ts @@ -10,9 +10,17 @@ * (2a) exit 0 + empty stdout โ†’ envelope_missing / protocol * (2b) non-zero exit + empty stdout โ†’ engine_exit_ / engine * (2c) partial/truncated JSON โ†’ engine_exit_ / engine (rule 2) + * + * Plus the protocol-0.4.0 additions: the envelope's identity and usage block + * are surfaced rather than discarded, and `stderrTailBytes` bounds the tail in + * real UTF-8 BYTES. */ import { describe, it, expect } from "vitest"; -import { parseRunOutput } from "../src/run-output-parser.js"; +import { + parseRunOutput, + tailStderrBytes, + STDERR_TAIL_BYTES, +} from "../src/run-output-parser.js"; import type { SubprocessOutcome } from "../src/run-output-parser.js"; /** Helper to build a valid ยง4.1 envelope with overrides. */ @@ -20,18 +28,23 @@ function makeEnvelope( overrides: Record = {}, ): Record { const base: Record = { - protocolVersion: "0.2.0", + protocolVersion: "0.4.0", sessionId: "sess-abc-001", turnId: "turn-1", reply: "It is 2:15pm Pacific time.", error: null, metadata: { + // Protocol 0.4.0 usage block. tokensIn is the CHARGED total: + // 1247 = 900 new + 300 cache reads + 47 cache writes. tokensIn: 1247, tokensOut: 89, + cacheReadTokens: 300, + cacheWriteTokens: 47, + costUsd: "0.00421500", durationMs: 1832, bundleDigest: "sha256:7f3a9e2b4c5d6e8f", - engineVersion: "0.2.0", - protocolVersion: "0.2.0", + engineVersion: "0.4.0", + protocolVersion: "0.4.0", correlationId: "01HXYZ123ABC456DEF789", }, }; @@ -50,6 +63,9 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.type).toBe("result"); if (ev.type === "result") { expect(ev.text).toBe("hello world"); + expect(ev.sessionId).toBe("sess-abc-001"); + expect(ev.turnId).toBe("turn-1"); + expect(ev.exitCode).toBe(0); } }); @@ -65,6 +81,9 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.type).toBe("result"); if (ev.type === "result") { expect(ev.text).toBe("envelope-wins"); + // Reported as observed, not used to override the envelope: this is how a + // host sees a post-flush crash without the outcome being mis-classified. + expect(ev.exitCode).toBe(1); } }); @@ -81,12 +100,15 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { stderrTail: "Traceback (most recent call last):\n ...", }, metadata: { - tokensIn: 0, + tokensIn: 512, tokensOut: 0, + cacheReadTokens: 0, + cacheWriteTokens: 0, + costUsd: null, durationMs: 247, bundleDigest: "sha256:7f3a9e2b", - engineVersion: "0.2.0", - protocolVersion: "0.2.0", + engineVersion: "0.4.0", + protocolVersion: "0.4.0", correlationId: "01HXYZ123ABC456DEF789", }, }); @@ -105,6 +127,18 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.message).toContain("failed to translate"); expect(ev.stderrTail).toContain("Traceback"); expect(ev.retryable).toBe(false); + // D6: a turn that spent tokens and THEN failed still reports what it + // spent. Nothing else on the failure path carries usage. + expect(ev.sessionId).toBe("sess-abc-001"); + expect(ev.turnId).toBe("turn-1"); + expect(ev.exitCode).toBe(3); + expect(ev.usage).toEqual({ + inputTokens: 512, + outputTokens: 0, + cacheReadTokens: 0, + cacheWriteTokens: 0, + costUsd: null, + }); } }); @@ -122,6 +156,11 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.severity).toBe("error"); expect(ev.retryable).toBe(false); expect(ev.message).toMatch(/envelope/i); + // Rule 2: no envelope means no identity and no usage to report. + expect(ev.sessionId).toBeUndefined(); + expect(ev.turnId).toBeUndefined(); + expect(ev.usage).toBeUndefined(); + expect(ev.exitCode).toBe(0); } }); @@ -140,13 +179,15 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.severity).toBe("error"); expect(ev.retryable).toBe(false); expect(ev.stderrTail).toBe(stderr); + expect(ev.exitCode).toBe(137); + expect(ev.usage).toBeUndefined(); } }); it("(2c) partial/truncated JSON falls to rule 2 (engine_exit_, classification engine)", () => { // Per ยง4.4 rule 2: belt-and-suspenders โ€” partial JSON is NOT half-parsed. const outcome: SubprocessOutcome = { - stdout: '{"protocolVersion":"0.2.0","sessionId":"sess-abc","turnId":"turn-1","reply":"hi"', + stdout: '{"protocolVersion":"0.4.0","sessionId":"sess-abc","turnId":"turn-1","reply":"hi"', stderr: "engine died mid-write\n", exitCode: 1, }; @@ -157,10 +198,14 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.classification).toBe("engine"); expect(ev.severity).toBe("error"); expect(ev.retryable).toBe(false); + // The sessionId is textually present in that truncated stdout, but the + // envelope did not parse, so claiming it would be reading a half-parsed + // envelope โ€” exactly what rule 2 forbids. + expect(ev.sessionId).toBeUndefined(); } }); - it("truncates stderrTail to 4096 chars on synthesized engine errors", () => { + it("truncates stderrTail to 4096 BYTES on synthesized engine errors", () => { // stderr longer than 4096 bytes โ€” only the last 4096 should be kept. const long = "X".repeat(5000) + "TAIL_MARKER"; const outcome: SubprocessOutcome = { @@ -172,9 +217,278 @@ describe("parseRunOutput โ€” ยง4.1 envelope + SC-D precedence", () => { expect(ev.type).toBe("error"); if (ev.type === "error") { expect(ev.stderrTail).toBeDefined(); - expect(ev.stderrTail!.length).toBe(4096); + expect(Buffer.byteLength(ev.stderrTail!, "utf-8")).toBe(4096); // Last bytes must be preserved (we keep the *tail*). expect(ev.stderrTail!.endsWith("TAIL_MARKER")).toBe(true); } }); }); + +describe("parseRunOutput โ€” fallbackSessionId on synthesized (Rule 2) paths", () => { + it("(f1) envelope_missing carries the caller's sessionId, and still no turnId", () => { + const ev = parseRunOutput( + { stdout: "", stderr: "", exitCode: 0 }, + { fallbackSessionId: "sess-host-42" }, + ); + expect(ev.type).toBe("error"); + if (ev.type !== "error") return; + expect(ev.code).toBe("envelope_missing"); + expect(ev.sessionId).toBe("sess-host-42"); + // The engine assigns turn ids and no envelope came back, so there is + // genuinely nothing to report here. Absent, never invented. + expect(ev.turnId).toBeUndefined(); + expect(ev.usage).toBeUndefined(); + }); + + it("(f2) engine_exit_ carries the caller's sessionId, and still no turnId", () => { + const ev = parseRunOutput( + { stdout: "", stderr: "boom\n", exitCode: 7 }, + { fallbackSessionId: "sess-host-42" }, + ); + expect(ev.type).toBe("error"); + if (ev.type !== "error") return; + expect(ev.code).toBe("engine_exit_7"); + expect(ev.sessionId).toBe("sess-host-42"); + expect(ev.turnId).toBeUndefined(); + expect(ev.exitCode).toBe(7); + }); + + it("(f3) partial JSON still falls to rule 2, reporting the caller's id โ€” not the one in the unparsed text", () => { + const ev = parseRunOutput( + { + stdout: + '{"protocolVersion":"0.4.0","sessionId":"sess-from-broken-json","turnId":"turn-1","reply":"hi"', + stderr: "engine died mid-write\n", + exitCode: 1, + }, + { fallbackSessionId: "sess-host-42" }, + ); + expect(ev.type).toBe("error"); + if (ev.type !== "error") return; + expect(ev.code).toBe("engine_exit_1"); + // The id comes from the handle, NOT from half-parsing a broken envelope. + expect(ev.sessionId).toBe("sess-host-42"); + expect(ev.turnId).toBeUndefined(); + }); + + it("(f4) omitting the option leaves sessionId absent (unchanged default)", () => { + const ev = parseRunOutput({ stdout: "", stderr: "", exitCode: 3 }); + expect(ev.type).toBe("error"); + if (ev.type !== "error") return; + expect(ev.sessionId).toBeUndefined(); + expect(ev.turnId).toBeUndefined(); + }); + + it("(f5) never overrides a parsed envelope โ€” Rule 1 result keeps the envelope's ids", () => { + const ev = parseRunOutput( + { stdout: JSON.stringify(makeEnvelope()), stderr: "", exitCode: 0 }, + { fallbackSessionId: "sess-host-42" }, + ); + expect(ev.type).toBe("result"); + if (ev.type !== "result") return; + expect(ev.sessionId).toBe("sess-abc-001"); + expect(ev.turnId).toBe("turn-1"); + }); + + it("(f6) never overrides a parsed envelope โ€” Rule 1 error keeps the envelope's ids", () => { + const env = makeEnvelope({ + error: { code: "provider_auth_failed", classification: "engine" }, + }); + const ev = parseRunOutput( + { stdout: JSON.stringify(env), stderr: "", exitCode: 1 }, + { fallbackSessionId: "sess-host-42" }, + ); + expect(ev.type).toBe("error"); + if (ev.type !== "error") return; + expect(ev.code).toBe("provider_auth_failed"); + expect(ev.sessionId).toBe("sess-abc-001"); + expect(ev.turnId).toBe("turn-1"); + }); +}); + +describe("parseRunOutput โ€” usage block (protocol 0.4.0)", () => { + it("surfaces the metadata usage block verbatim, with no wrapper-side arithmetic", () => { + const outcome: SubprocessOutcome = { + stdout: JSON.stringify(makeEnvelope()), + stderr: "", + exitCode: 0, + }; + const ev = parseRunOutput(outcome); + if (ev.type !== "result") throw new Error("expected result event"); + + // tokensIn is copied straight through as the CHARGED total. The wrapper + // must NOT re-add cache reads/writes: the engine already did. + expect(ev.usage).toEqual({ + inputTokens: 1247, + outputTokens: 89, + cacheReadTokens: 300, + cacheWriteTokens: 47, + costUsd: "0.00421500", + }); + }); + + it("keeps costUsd a STRING, never a number (monetary precision)", () => { + const outcome: SubprocessOutcome = { + stdout: JSON.stringify( + makeEnvelope({ + metadata: { tokensIn: 1, tokensOut: 1, costUsd: "0.10000000000000001" }, + }), + ), + stderr: "", + exitCode: 0, + }; + const ev = parseRunOutput(outcome); + if (ev.type !== "result") throw new Error("expected result event"); + expect(typeof ev.usage?.costUsd).toBe("string"); + // Round-tripping through `number` would collapse this to 0.1. + expect(ev.usage?.costUsd).toBe("0.10000000000000001"); + }); + + it("reports costUsd null (not 0) when no provider reported a cost", () => { + const outcome: SubprocessOutcome = { + stdout: JSON.stringify( + makeEnvelope({ metadata: { tokensIn: 10, tokensOut: 2, costUsd: null } }), + ), + stderr: "", + exitCode: 0, + }; + const ev = parseRunOutput(outcome); + if (ev.type !== "result") throw new Error("expected result event"); + expect(ev.usage?.costUsd).toBeNull(); + }); + + it("omits usage entirely when metadata carries no usage keys (pre-0.4.0 engine)", () => { + // "Not reported" and "reported as zero" are different claims. An engine + // that never had a usage block must not be made to look like it spent 0. + const outcome: SubprocessOutcome = { + stdout: JSON.stringify( + makeEnvelope({ metadata: { durationMs: 12, correlationId: "c" } }), + ), + stderr: "", + exitCode: 0, + }; + const ev = parseRunOutput(outcome); + if (ev.type !== "result") throw new Error("expected result event"); + expect(ev.usage).toBeUndefined(); + }); + + it("coerces a malformed token count to 0 rather than throwing", () => { + const outcome: SubprocessOutcome = { + stdout: JSON.stringify( + makeEnvelope({ metadata: { tokensIn: "not-a-number", tokensOut: 5 } }), + ), + stderr: "", + exitCode: 0, + }; + const ev = parseRunOutput(outcome); + if (ev.type !== "result") throw new Error("expected result event"); + expect(ev.usage?.inputTokens).toBe(0); + expect(ev.usage?.outputTokens).toBe(5); + }); +}); + +describe("stderrTailBytes โ€” the tail is bounded in BYTES, not characters", () => { + const outcomeWith = (stderr: string): SubprocessOutcome => ({ + stdout: JSON.stringify(makeEnvelope()), + stderr, + exitCode: 0, + }); + + it("defaults to STDERR_TAIL_BYTES when the option is omitted", () => { + const ev = parseRunOutput(outcomeWith("y".repeat(9000))); + if (ev.type !== "result") throw new Error("expected result event"); + expect(Buffer.byteLength(ev.stderrTail!, "utf-8")).toBe(STDERR_TAIL_BYTES); + }); + + it("null keeps the ENTIRE buffer", () => { + const stderr = "z".repeat(9000); + const ev = parseRunOutput(outcomeWith(stderr), { stderrTailBytes: null }); + if (ev.type !== "result") throw new Error("expected result event"); + expect(ev.stderrTail).toBe(stderr); + }); + + it("0 disables capture", () => { + const ev = parseRunOutput(outcomeWith("plenty of stderr"), { + stderrTailBytes: 0, + }); + if (ev.type !== "result") throw new Error("expected result event"); + expect(ev.stderrTail).toBeUndefined(); + }); + + it("0 also suppresses a tail the envelope supplied", () => { + // The knob is a host instruction about the FIELD, not about one source of + // it: "disabled" that still returns stderr would be a lie. + const env = makeEnvelope({ + reply: "", + error: { code: "boom", stderrTail: "engine-supplied tail" }, + }); + const ev = parseRunOutput( + { stdout: JSON.stringify(env), stderr: "", exitCode: 1 }, + { stderrTailBytes: 0 }, + ); + if (ev.type !== "error") throw new Error("expected error event"); + expect(ev.stderrTail).toBeUndefined(); + }); + + it("caps a multibyte tail in bytes and never splits a codepoint", () => { + // 3 bytes per character in UTF-8. A character-counting implementation + // returns ~3x the cap here; a naive byte slice returns U+FFFD. + const stderr = "ใƒใƒŠใƒŠ่ฒฟๆ˜“ใฎๆญดๅฒ".repeat(200); + const ev = parseRunOutput(outcomeWith(stderr), { stderrTailBytes: 512 }); + if (ev.type !== "result") throw new Error("expected result event"); + + const tail = ev.stderrTail!; + expect(tail).toBeDefined(); + expect(/^[\x00-\x7F]*$/.test(tail)).toBe(false); // genuinely multibyte + expect(Buffer.byteLength(tail, "utf-8")).toBeLessThanOrEqual(512); + // 512 is not a multiple of 3, so the trim to a codepoint boundary must + // have dropped 1-2 bytes: proof the boundary logic actually ran. + expect(Buffer.byteLength(tail, "utf-8")).toBeGreaterThan(512 - 3); + expect(tail).not.toContain("\uFFFD"); + // The tail is the END of the buffer. + expect(stderr.endsWith(tail)).toBe(true); + }); +}); + +describe("tailStderrBytes โ€” direct unit coverage", () => { + it("returns undefined for an empty string regardless of limit", () => { + expect(tailStderrBytes("", 100)).toBeUndefined(); + expect(tailStderrBytes("", null)).toBeUndefined(); + expect(tailStderrBytes("", 0)).toBeUndefined(); + }); + + it("returns the text unchanged when it already fits the cap", () => { + expect(tailStderrBytes("short", 4096)).toBe("short"); + }); + + it("measures the cap in UTF-8 bytes, not UTF-16 code units", () => { + // 4 characters, 12 UTF-8 bytes. A cap of 6 bytes must yield 2 characters + // (6 bytes), not 6 characters. + const text = "ใ‚ใ„ใ†ใˆ"; + const tail = tailStderrBytes(text, 6)!; + expect(Buffer.byteLength(tail, "utf-8")).toBe(6); + expect(tail).toBe("ใ†ใˆ"); + }); + + it("backs up to a codepoint boundary rather than emitting U+FFFD", () => { + const text = "ใ‚ใ„ใ†ใˆ"; // 12 bytes + const tail = tailStderrBytes(text, 7)!; // 7 is mid-codepoint + expect(tail).toBe("ใ†ใˆ"); // 6 bytes โ€” one fewer than the cap, on purpose + expect(Buffer.byteLength(tail, "utf-8")).toBe(6); + expect(tail).not.toContain("\uFFFD"); + }); + + it("handles a 4-byte codepoint (astral plane) at the boundary", () => { + const text = "ab๐Ÿ˜€๐Ÿ˜€"; // 2 + 4 + 4 = 10 bytes + const tail = tailStderrBytes(text, 5)!; // lands inside the last emoji + expect(tail).toBe("๐Ÿ˜€"); + expect(Buffer.byteLength(tail, "utf-8")).toBe(4); + expect(tail).not.toContain("\uFFFD"); + }); + + it("returns undefined when the cap trims away every whole codepoint", () => { + // A 2-byte cap cannot hold a 3-byte character; the boundary walk consumes + // the whole window and the honest answer is an empty tail. + expect(tailStderrBytes("ใ‚ใ„ใ†ใˆ", 2)).toBe(""); + }); +}); diff --git a/wrappers/typescript/test/session-mode-a-shape.test.ts b/wrappers/typescript/test/session-mode-a-shape.test.ts index 27a98769..f6b2cb67 100644 --- a/wrappers/typescript/test/session-mode-a-shape.test.ts +++ b/wrappers/typescript/test/session-mode-a-shape.test.ts @@ -6,14 +6,23 @@ * export type DisplayEvent = * | { type: 'init'; sessionId: string } * | { type: 'activity' } - * | { type: 'result'; text: string } + * | { type: 'result'; text: string; + * sessionId: string; turnId: string; exitCode: number; + * usage?: Usage; stderrTail?: string } * | { type: 'error'; code: string; * classification: 'transport' | 'protocol' | 'engine' | 'approval' | 'unknown'; * severity: 'error' | 'warning'; * correlationId: string; * message: string; * stderrTail?: string; - * retryable: boolean } + * retryable: boolean; + * sessionId?: string; turnId?: string; + * exitCode?: number; usage?: Usage } + * + * The identity / usage fields on the terminal variants arrived with protocol + * 0.4.0. They are REQUIRED on `result` (which only exists when a ยง4.1 envelope + * parsed) and OPTIONAL on `error` (which can be synthesized with no envelope). + * What CR-C removed and this test still guards is the opaque `payload` bag. * * Why this test must FAIL at the RED step: * @@ -102,26 +111,45 @@ describe("DisplayEvent โ€” simplified Mode A shape (CR-C)", () => { expect(Object.keys(ev)).toEqual(["type"]); }); - it("(iii) result event carries `text` only โ€” no payload", () => { - const ev: DisplayEvent = { type: "result", text: "hello from engine" }; + it("(iii) result event carries text + envelope identity โ€” still no payload", () => { + // Protocol 0.4.0: the result variant additionally carries the identity the + // ยง4.1 envelope always supplies (sessionId, turnId) plus the observed + // exitCode. `usage` and `stderrTail` stay optional. The opaque `payload` + // bag CR-C removed is still gone โ€” that is what this case guards. + const ev: DisplayEvent = { + type: "result", + text: "hello from engine", + sessionId: "sess-abc", + turnId: "turn-1", + exitCode: 0, + }; if (ev.type !== "result") { throw new Error("expected result branch"); } expect(ev.text).toBe("hello from engine"); + expect(ev.sessionId).toBe("sess-abc"); + expect(ev.turnId).toBe("turn-1"); + expect(ev.exitCode).toBe(0); + + // usage / stderrTail are optional โ€” absent here. + expect(ev.usage).toBeUndefined(); + expect(ev.stderrTail).toBeUndefined(); // payload must not exist on the result variant under the new shape. // @ts-expect-error - payload is not a property of the result variant (CR-C). const payloadProbe = ev.payload; - // @ts-expect-error - turnId is not a property of the result variant. - const turnIdProbe = ev.turnId; - expect(payloadProbe).toBeUndefined(); - expect(turnIdProbe).toBeUndefined(); - // Structural assertion: only `type` and `text`. - expect(Object.keys(ev).sort()).toEqual(["text", "type"]); + // Structural assertion: exactly the required keys, nothing opaque. + expect(Object.keys(ev).sort()).toEqual([ + "exitCode", + "sessionId", + "text", + "turnId", + "type", + ]); }); it("(iv) error event carries code, classification='engine', severity, correlationId, message, retryable=false", () => { @@ -149,13 +177,18 @@ describe("DisplayEvent โ€” simplified Mode A shape (CR-C)", () => { // stderrTail is optional โ€” absent here. expect(ev.stderrTail).toBeUndefined(); - // No payload / turnId on the error variant. + // Protocol 0.4.0 envelope-derived fields are OPTIONAL on the error variant + // precisely because an error event can be synthesized with no envelope at + // all (Rule 2). This literal is such a case, so they are absent. + expect(ev.sessionId).toBeUndefined(); + expect(ev.turnId).toBeUndefined(); + expect(ev.exitCode).toBeUndefined(); + expect(ev.usage).toBeUndefined(); + + // No opaque payload bag on the error variant. // @ts-expect-error - payload is not a property of the error variant (CR-C). const payloadProbe = ev.payload; - // @ts-expect-error - turnId is not a property of the error variant. - const turnIdProbe = ev.turnId; expect(payloadProbe).toBeUndefined(); - expect(turnIdProbe).toBeUndefined(); }); it("(v) discriminated-union exhaustiveness โ€” switch on ev.type narrows each branch", () => { @@ -166,7 +199,7 @@ describe("DisplayEvent โ€” simplified Mode A shape (CR-C)", () => { const events: DisplayEvent[] = [ { type: "init", sessionId: "s" }, { type: "activity" }, - { type: "result", text: "r" }, + { type: "result", text: "r", sessionId: "s", turnId: "t", exitCode: 0 }, { type: "error", code: "engine_exit_1", diff --git a/wrappers/typescript/test/session-subprocess.test.ts b/wrappers/typescript/test/session-subprocess.test.ts index d68cac8a..9d4efe62 100644 --- a/wrappers/typescript/test/session-subprocess.test.ts +++ b/wrappers/typescript/test/session-subprocess.test.ts @@ -148,7 +148,16 @@ describe("SessionHandle (Mode A v2 subprocess driver, ยง5.2)", () => { expect(events[0]).toEqual({ type: "init", sessionId: "sess-test" }); const result = events[events.length - 1]; - expect(result).toEqual({ type: "result", text: "hello from fake engine" }); + expect(result).toMatchObject({ + type: "result", + text: "hello from fake engine", + }); + // Protocol 0.4.0: the identity fields the envelope carries are surfaced on + // the terminal event rather than discarded by the parser. + if (result?.type !== "result") throw new Error("expected result event"); + expect(typeof result.sessionId).toBe("string"); + expect(typeof result.turnId).toBe("string"); + expect(result.exitCode).toBe(0); // No "error" variants in the stream. for (const ev of events) { @@ -326,3 +335,80 @@ describe("SessionHandle (Mode A v2 subprocess driver, ยง5.2)", () => { expect(last.classification).toBe("transport"); }, 10000); }); + +/** + * Identity on the synthesized (Rule 2) failure paths. + * + * No envelope came back on any of these, so there is nothing to READ the + * identity from โ€” but the handle was handed a sessionId at construction time, + * so it knows one anyway and a host correlating the failure needs it. The + * turnId is the opposite case: the engine assigns it, nothing came back, and + * inventing one would be a fabrication. + */ +describe("SessionHandle โ€” synthesized failures carry the session id, never a turn id", () => { + it("(m) spawn_failed reports the handle's sessionId and no turnId", async () => { + const handle = new SessionHandle( + makeParams({ + binaryPath: join(workDir, "this-file-does-not-exist"), + sessionId: "sess-synth-1", + }), + ); + const events = await drain(handle.submit("anything")); + + const last = events[events.length - 1]; + expect(last?.type).toBe("error"); + if (last?.type !== "error") return; + expect(last.code).toBe("spawn_failed"); + expect(last.sessionId).toBe("sess-synth-1"); + expect(last.turnId).toBeUndefined(); + }, 10000); + + it("(n) engine_hung reports the handle's sessionId and no turnId", async () => { + const handle = new SessionHandle( + makeParams({ + binaryPath: sleepBin, + sessionId: "sess-synth-2", + timeoutMs: 250, + }), + ); + const events = await drain(handle.submit("never returns")); + + const last = events[events.length - 1]; + expect(last?.type).toBe("error"); + if (last?.type !== "error") return; + expect(last.code).toBe("engine_hung"); + expect(last.sessionId).toBe("sess-synth-2"); + expect(last.turnId).toBeUndefined(); + }, 10000); + + it("(o) engine_exit_ from the parser reports the handle's sessionId and no turnId", async () => { + // Proves the parser's Rule 2 path is reached WITH the fallback wired in by + // SessionHandle, not just when parseRunOutput is called directly. + const handle = new SessionHandle( + makeParams({ binaryPath: exitBin, sessionId: "sess-synth-3" }), + ); + const events = await drain(handle.submit("boom")); + + const last = events[events.length - 1]; + expect(last?.type).toBe("error"); + if (last?.type !== "error") return; + expect(last.code).toBe("engine_exit_7"); + expect(last.sessionId).toBe("sess-synth-3"); + expect(last.turnId).toBeUndefined(); + }, 10000); + + it("(p) a parsed envelope still wins: result keeps the ENVELOPE's ids, not the handle's", async () => { + // echoBin emits sessionId "sess-test" / turnId "turn-test" regardless of + // argv, so a mismatched handle id proves the envelope is authoritative. + const handle = new SessionHandle( + makeParams({ binaryPath: echoBin, sessionId: "sess-handle-mismatch" }), + ); + const events = await drain(handle.submit("hi")); + + const last = events[events.length - 1]; + expect(last?.type).toBe("result"); + if (last?.type !== "result") return; + expect(last.sessionId).toBe("sess-test"); + expect(last.turnId).toBe("turn-test"); + }, 10000); +}); diff --git a/wrappers/typescript/test/smoke.test.ts b/wrappers/typescript/test/smoke.test.ts index 22f1880a..1101d2a2 100644 --- a/wrappers/typescript/test/smoke.test.ts +++ b/wrappers/typescript/test/smoke.test.ts @@ -3,6 +3,6 @@ import { PROTOCOL_VERSION_REQUIRED_BY_WRAPPER } from "../src/index.js"; describe("smoke", () => { it("exports the correct protocol version constant", () => { - expect(PROTOCOL_VERSION_REQUIRED_BY_WRAPPER).toBe("0.3.0"); + expect(PROTOCOL_VERSION_REQUIRED_BY_WRAPPER).toBe("0.4.0"); }); }); From fd16fdce45654ff83ed898c46d7280c2dd14efe6 Mon Sep 17 00:00:00 2001 From: David Koleczek <45405824+DavidKoleczek@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:03:45 -0700 Subject: [PATCH 2/3] fix(usage): tokensIn was double-counting cache reads charged_input summed inputTokens + cacheReadTokens + cacheWriteTokens on the assumption that the three are disjoint buckets. They are not. amplifier-core's docs/contracts/PROVIDER_CONTRACT.md specifies a provider's input_tokens as the "gross total (fresh + cache_read combined)", and providers normalize to that shape: the Anthropic module ADDS cache_read_input_tokens into input_tokens, and the OpenAI module subtracts only cache_write out of the vendor total, leaving cache reads inside. cacheReadTokens is therefore a reported SUBSET of the gross figure, not an addend. Adding it a second time roughly doubles the reported input on a cache-heavy turn -- the normal case in an agent loop. On the turn measured while building this, tokensIn read 24,700 against a true charged input of 12,788. That is the same class of error the change set out to fix, inverted: it replaced a wrong zero with a wrong non-zero, which is worse, because a zero is visibly broken and a plausible number gets persisted downstream. charged_input is now inputTokens + cacheWriteTokens -- cache writes are the one bucket billed on top of the gross total. This matches _compute_total_input in amplifier-module-hooks-streaming-ui, the ecosystem's existing consumer of the same event. Renamed new_input to gross_input so the attribute stops asserting the thing that was wrong. The e2e oracle carried the identical assumption, so tokensIn was being checked against a second implementation of the same wrong formula and passed. Corrected, and its independence from UsageAccumulator is now stated as deliberate: if both sides shared a helper the suite could only prove the accumulator summed events correctly and would be structurally blind to the formula itself. Also documents why costUsd sums per-call cost rather than substituting sessionCostTotal: this engine never calls bridge_child_cost, so delegated sub-agent spend never reaches the parent's session.cost channel, while child sessions do inherit display.emit and emit their per-call cost events here. The display stream is the only path that sees sub-agent cost today. Two doc corrections in the same pass: tokensIn had no prior definition in main to "match", and the protocol version check is exact string equality, so hosts cannot gate on the new fields -- a version mismatch is refused outright. No behavior change outside charged_input. Wrapper edits are comment-only. --- CHANGELOG.md | 6 +- docs/spec/envelope-and-errors.md | 11 ++-- docs/spec/wrapper-contract.md | 4 +- src/amplifier_agent_lib/protocol/methods.py | 28 +++++--- .../protocol_points/usage_accumulator.py | 66 +++++++++++++------ tests/e2e/suites/usage/events_oracle.py | 28 +++++--- tests/e2e/suites/usage/test_usage_envelope.py | 26 ++++---- tests/e2e/suites/usage/test_usage_wrapper.py | 5 +- .../python-py/src/amplifier_agent_py/types.py | 13 ++-- wrappers/typescript/dist/session.d.ts | 19 ++++-- wrappers/typescript/src/session.ts | 19 ++++-- .../typescript/test/run-output-parser.test.ts | 6 +- 12 files changed, 151 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1613dd1..dc937381 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 of the picture: `cacheReadTokens` and `cacheWriteTokens` (both `int`), and `costUsd` (a decimal STRING, e.g. `"0.00842"`, or `null` when no provider reported a cost -- never a float, since a float cannot hold a decimal money value exactly and a host summing per-turn costs from floats - accumulates drift it cannot see). `tokensIn` is the CHARGED input total: new input plus both - cache fields; a host wanting the new-only figure derives it as + accumulates drift it cannot see). `tokensIn` is the CHARGED input total: the provider's gross + input plus `cacheWriteTokens`. Cache reads are already counted inside the gross input per + amplifier-core's `PROVIDER_CONTRACT.md`, so `cacheReadTokens` is a subset of `tokensIn` rather + than an addend; a host wanting the fresh-only figure derives it as `tokensIn - cacheReadTokens - cacheWriteTokens`. Usage accounting sits upstream of the CLI's display renderer, so the same numbers are reported under `--display text`, `--display ndjson`, and `--quiet` alike. diff --git a/docs/spec/envelope-and-errors.md b/docs/spec/envelope-and-errors.md index f8b5b700..cd9df412 100644 --- a/docs/spec/envelope-and-errors.md +++ b/docs/spec/envelope-and-errors.md @@ -46,10 +46,13 @@ Under `--output text` stdout is left intact so a human sees the reply as it is p - `sessionId` echoes `--session-id` when supplied, else the session id the engine assigned. - `turnId` is the engine's turn id, defaulting to `"turn-1"`. One turn is submitted per process, so in practice it is always `turn-1`. -- `tokensIn` is the CHARGED input total for the turn: new input tokens plus `cacheReadTokens` plus - `cacheWriteTokens`. The model sees all three as input; the split is a billing distinction only, - so reporting new-input alone would understate a cached turn by orders of magnitude. A caller - that wants the new-only figure derives it as `tokensIn - cacheReadTokens - cacheWriteTokens`. +- `tokensIn` is the CHARGED input total for the turn: the provider's gross input plus + `cacheWriteTokens`. Per amplifier-core's `PROVIDER_CONTRACT.md`, a provider's `input_tokens` is + already the gross total (fresh tokens and cache reads combined), so `cacheReadTokens` is a + **reported subset of `tokensIn`, not an addend** -- adding it in would double-count it and + roughly double the figure on a cache-heavy turn. `cacheWriteTokens` is the one bucket billed on + top of the gross total, so it is added. A caller that wants the fresh-only figure derives it as + `tokensIn - cacheReadTokens - cacheWriteTokens`. - `tokensOut`, `cacheReadTokens`, `cacheWriteTokens` are turn-scoped sums across every LLM call the turn made, including calls made by delegated sub-agents. They are independent of `--display` and of verbosity (`--quiet`, `-v`, `--debug`): the same numbers are reported no matter which renderer, diff --git a/docs/spec/wrapper-contract.md b/docs/spec/wrapper-contract.md index 9df3740f..35989929 100644 --- a/docs/spec/wrapper-contract.md +++ b/docs/spec/wrapper-contract.md @@ -188,8 +188,8 @@ host. `metadata` block with no wrapper-side arithmetic -- the engine already summed the turn: ``` -input_tokens / inputTokens mirrors metadata.tokensIn: the CHARGED total (new input + - cache reads + cache writes) +input_tokens / inputTokens mirrors metadata.tokensIn: the CHARGED total (gross input + + cache writes; cache reads are already inside gross) output_tokens / outputTokens mirrors metadata.tokensOut cache_read_tokens / cacheReadTokens mirrors metadata.cacheReadTokens cache_write_tokens / cacheWriteTokens mirrors metadata.cacheWriteTokens diff --git a/src/amplifier_agent_lib/protocol/methods.py b/src/amplifier_agent_lib/protocol/methods.py index 5d1264f6..b48d6d0b 100644 --- a/src/amplifier_agent_lib/protocol/methods.py +++ b/src/amplifier_agent_lib/protocol/methods.py @@ -16,10 +16,14 @@ ``cacheWriteTokens``, ``costUsd``), accumulated by the engine off the display event stream. The CLI's ``--output json`` envelope carries the same five fields in ``metadata`` on both the success and the error path. - ``tokensIn`` is the CHARGED input total (new + cache reads + cache - writes); ``costUsd`` is a decimal STRING or null, never a float. - Purely additive: no existing field changed shape or meaning, but the - minor version moves so hosts can gate on the new fields being present. + ``tokensIn`` is the CHARGED input total (gross input + cache writes; + cache reads are already inside the gross figure and are NOT added + again); ``costUsd`` is a decimal STRING or null, never a float. + Additive in shape -- no existing field changed meaning -- but NOT + optional for hosts: the version check is exact string equality + (``wrapper == engine``), so an engine and a wrapper on different + protocol versions REFUSE each other rather than negotiating down. + Engine and both wrapper SDKs must therefore be released together. 0.2.0 โ€” MCP config delivery changed from inline ``mcpServers`` dict to a path string (``mcpConfigPath``) pointing at a JSON file in the format documented by amplifier-module-tool-mcp (top-level ``mcpServers`` key). @@ -131,16 +135,20 @@ class TurnSubmitResult(TypedDict): reply: str | None turnId: str sessionId: str # SC-6 - #: CHARGED input tokens: new input + cache reads + cache writes. The model - #: sees all three as input; the split is a billing distinction only, so - #: reporting new-input alone understates a cached turn by orders of - #: magnitude. Derive new-input as tokensIn - cacheReadTokens - cacheWriteTokens. + #: CHARGED input tokens: gross input + cache writes. Per amplifier-core's + #: PROVIDER_CONTRACT, a provider's input_tokens is ALREADY the gross total + #: (fresh + cache reads combined), so cacheReadTokens is a reported subset of + #: it, not a separate bucket -- adding it again roughly doubles a cache-heavy + #: turn. Cache writes are the one bucket billed on top of the gross total. + #: Derive fresh input as tokensIn - cacheReadTokens - cacheWriteTokens. tokensIn: int #: Output tokens generated across the turn. tokensOut: int - #: The portion of tokensIn that was served from the provider's prompt cache. + #: The portion of tokensIn already counted in gross input that the provider + #: served from its prompt cache. Reported for visibility; never added on top. cacheReadTokens: int - #: The portion of tokensIn that was written into the provider's prompt cache. + #: Tokens written into the provider's prompt cache. Billed on top of gross + #: input, so this IS a component of tokensIn rather than a subset of it. cacheWriteTokens: int #: Turn cost in USD as a decimal STRING (e.g. "0.0123"), or None when no #: provider reported a cost. A string, never a float: a float cannot hold a diff --git a/src/amplifier_agent_lib/protocol_points/usage_accumulator.py b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py index e45dd177..b5a6e522 100644 --- a/src/amplifier_agent_lib/protocol_points/usage_accumulator.py +++ b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py @@ -19,21 +19,38 @@ The decorator is an **observer, never a filter**. Every event is forwarded to the wrapped system unchanged, in order, whether or not this class understood it. -Arithmetic notes (mirrors ``amplifier_agent_http/_event_translator.py``, which was -already forced to correct to exactly this): - -* Input tokens arrive split across three buckets -- ``inputTokens`` (new, full - rate), ``cacheWriteTokens`` (~1.25x) and ``cacheReadTokens`` (~0.1x). The model - sees all three as input; the split is purely a billing distinction. Reporting - only ``inputTokens`` made cached turns look 1000-2000x cheaper than they were. - ``charged_input`` is therefore the sum of all three. +Arithmetic notes: + +* ``inputTokens`` is already the GROSS input total -- fresh tokens plus any tokens + read back from the prompt cache. This is normative: amplifier-core's + ``docs/contracts/PROVIDER_CONTRACT.md`` specifies ``input_tokens`` as the "gross + total (fresh + cache_read combined)", and providers normalize to it (the + Anthropic module ADDS ``cache_read_input_tokens`` into ``input_tokens``; the + OpenAI module subtracts only ``cache_write`` out of the vendor total, leaving + cache reads in). So ``cacheReadTokens`` is a REPORTED SUBSET of ``inputTokens``, + not a disjoint bucket -- adding it to ``inputTokens`` double-counts it, roughly + doubling the reported figure on a cache-heavy turn. +* ``cacheWriteTokens`` is the exception: cache creation is billed on top of the + gross total and is NOT included in ``inputTokens``. So the charged input is + ``inputTokens + cacheWriteTokens``, and nothing else. This matches the formula + the ecosystem's own display consumer uses + (``amplifier-module-hooks-streaming-ui``, ``_compute_total_input``). * Usage events are **summed**, never taken-last. The engine emits a trailing rollup event with ``inputTokens: 0`` / ``outputTokens: 0`` that carries only ``sessionCostTotal`` (``hook_streaming.on_orchestrator_complete``); a last-event-wins reader reports zero for the whole turn. -* ``sessionCostTotal`` is deliberately **not** added to ``cost_usd``. It is a - session-wide total collected from the kernel's cost contributions, not a - per-call cost, so adding it to a sum of per-call costs double-counts. +* ``sessionCostTotal`` is deliberately **not** added to ``cost_usd``, and is not + used in its place either. It is a session-wide total collected from the kernel's + ``session.cost`` channel, not a per-call cost, so adding it to a sum of per-call + costs double-counts. Substituting it would also LOSE cost: this engine never + calls ``amplifier_foundation.bridge_child_cost`` (see the MVP scope note in + ``spawn.py``), so delegated sub-agent spend never reaches the parent's + ``session.cost`` channel -- while child sessions DO inherit the parent's + ``display.emit`` capability and mount this same streaming hook, so their + per-call ``cost`` events do arrive here. Summing per-call cost off the display + stream is therefore the only path that sees sub-agent spend today. If the cost + bridge is ever wired up, revisit this -- do not switch to ``sessionCostTotal`` + before then. * ``cost`` crosses the wire as a decimal **string** to preserve monetary precision, and is parsed with ``Decimal``. Never float: summing per-call costs as binary floats accumulates drift a host cannot see. @@ -84,10 +101,12 @@ class UsageAccumulator: Attributes ---------- - new_input: - Sum of ``inputTokens`` -- input tokens billed at the full rate. + gross_input: + Sum of ``inputTokens`` -- the provider's GROSS input total, which already + includes whatever it served from the prompt cache. cache_read_tokens: - Sum of ``cacheReadTokens``. + Sum of ``cacheReadTokens`` -- the cached portion already counted inside + ``gross_input``. Reported for visibility; never added to it. cache_write_tokens: Sum of ``cacheWriteTokens``. output_tokens: @@ -100,7 +119,7 @@ class UsageAccumulator: def __init__(self, inner: DisplaySystem) -> None: self._inner = inner - self.new_input: int = 0 + self.gross_input: int = 0 self.cache_read_tokens: int = 0 self.cache_write_tokens: int = 0 self.output_tokens: int = 0 @@ -135,7 +154,7 @@ def _observe(self, event: DisplayEvent) -> None: # SUM, never take-last: the trailing sessionCostTotal rollup carries # zeroes for both token counts and would otherwise erase the turn. - self.new_input += _to_int(get("inputTokens")) + self.gross_input += _to_int(get("inputTokens")) self.cache_read_tokens += _to_int(get("cacheReadTokens")) self.cache_write_tokens += _to_int(get("cacheWriteTokens")) self.output_tokens += _to_int(get("outputTokens")) @@ -152,12 +171,17 @@ def _observe(self, event: DisplayEvent) -> None: @property def charged_input(self) -> int: - """Total input tokens CHARGED: new + cache reads + cache writes. + """Total input tokens CHARGED: gross input + cache writes. - The model saw all three as input. A host that wants the new-only figure - derives it as ``charged_input - cache_read_tokens - cache_write_tokens``. + ``gross_input`` already contains ``cache_read_tokens`` (PROVIDER_CONTRACT: + ``input_tokens`` is the "gross total (fresh + cache_read combined)"), so + cache reads are NOT added again -- doing so double-counts them. Cache + writes are the one bucket billed on top of the gross total, so they are. + + A host that wants the fresh-only figure derives it as + ``charged_input - cache_read_tokens - cache_write_tokens``. """ - return self.new_input + self.cache_read_tokens + self.cache_write_tokens + return self.gross_input + self.cache_write_tokens def totals(self) -> dict[str, Any]: """Return the totals under their wire names. @@ -177,7 +201,7 @@ def totals(self) -> dict[str, Any]: def reset(self) -> None: """Zero every total. Called at the start of each turn to turn-scope them.""" - self.new_input = 0 + self.gross_input = 0 self.cache_read_tokens = 0 self.cache_write_tokens = 0 self.output_tokens = 0 diff --git a/tests/e2e/suites/usage/events_oracle.py b/tests/e2e/suites/usage/events_oracle.py index 6dba515e..6df6d9df 100644 --- a/tests/e2e/suites/usage/events_oracle.py +++ b/tests/e2e/suites/usage/events_oracle.py @@ -11,9 +11,17 @@ and the kernel's ``usage`` sub-dict carries ``input_tokens``, ``output_tokens``, ``cache_read_tokens``, ``cache_write_tokens`` and (when the provider reports one) -``cost_usd``. Note ``input_tokens`` there is the NEW input only -- cache reads and -cache writes are counted separately, which is why the charged total is a sum of the -three rather than ``input_tokens`` alone. +``cost_usd``. Per amplifier-core's ``docs/contracts/PROVIDER_CONTRACT.md``, +``input_tokens`` there is the GROSS input total -- fresh tokens plus cache reads +already combined -- and ``cache_write_tokens`` is the only bucket billed on top of +it. So the charged total is ``input_tokens + cache_write_tokens``; ``cache_read_tokens`` +is a reported subset of ``input_tokens`` and adding it in would double-count. + +This derivation is written out HERE, independently, rather than imported from +``UsageAccumulator``. That is the point of an oracle: if both sides called the same +helper, this suite could only ever prove the accumulator summed events correctly and +would be structurally blind to the formula being wrong. Two independent readings that +agree mean something; one reading compared against itself does not. The file is summed INSIDE the container by a small ``python3 -c`` program and only the totals cross back to the host. ``events.jsonl`` lines can be very large (a single @@ -96,9 +104,9 @@ class ProviderUsage: """Per-turn provider usage summed from ``llm:response`` records. - ``input_tokens`` is the NEW input the provider billed as fresh; ``charged_input`` - adds the cached halves, which is what the envelope's ``tokensIn`` is specified to - report. + ``input_tokens`` is the provider's GROSS input (fresh + cache reads combined); + ``charged_input`` adds cache writes, which is what the envelope's ``tokensIn`` is + specified to report. """ input_tokens: int @@ -111,8 +119,12 @@ class ProviderUsage: @property def charged_input(self) -> int: - """CHARGED input total: new input + cache reads + cache writes.""" - return self.input_tokens + self.cache_read_tokens + self.cache_write_tokens + """CHARGED input total: gross input + cache writes. + + ``cache_read_tokens`` is deliberately absent: it is already counted inside + ``input_tokens`` per PROVIDER_CONTRACT, so adding it double-counts. + """ + return self.input_tokens + self.cache_write_tokens def resolve_session_dir(dtu_id: str, session_id: str, *, root: str = WORKSPACES_ROOT) -> str: diff --git a/tests/e2e/suites/usage/test_usage_envelope.py b/tests/e2e/suites/usage/test_usage_envelope.py index cca29367..9048081c 100644 --- a/tests/e2e/suites/usage/test_usage_envelope.py +++ b/tests/e2e/suites/usage/test_usage_envelope.py @@ -3,10 +3,10 @@ Contract under test: after a turn completes, the stdout envelope's ``metadata`` block reports what the turn actually cost. - tokensIn int CHARGED input total = new input + cache reads + cache writes + tokensIn int CHARGED input total = gross input + cache writes tokensOut int output tokens - cacheReadTokens int the cached half of tokensIn that was read back - cacheWriteTokens int the cached half of tokensIn that was written + cacheReadTokens int cached portion ALREADY counted inside gross input + cacheWriteTokens int cache creation, billed on top of gross input costUsd str | None decimal STRING, never a float; None when no provider reported a cost @@ -160,12 +160,13 @@ def test_usage_envelope_breakdown_fields(dtu_id: str) -> None: def test_usage_envelope_breakdown_consistent(dtu_id: str) -> None: - """``tokensIn`` is the charged total, so the cached halves cannot exceed it. + """``tokensIn`` is the charged total, so the cached parts cannot exceed it. - ``tokensIn`` is specified as new input + cache reads + cache writes. A host that - wants the "new" figure derives it by subtracting the two cache fields, so if the - parts ever exceed the whole that subtraction goes negative and every downstream - cost calculation is wrong in a way no type check would catch. + ``tokensIn`` is specified as gross input + cache writes, where the gross input + already contains the cache reads. A host that wants the "fresh" figure derives it + by subtracting the two cache fields, so if the parts ever exceed the whole that + subtraction goes negative and every downstream cost calculation is wrong in a way + no type check would catch. """ metadata = _metadata(_run_turn(dtu_id, _session_id("usage-consistent"), [])) @@ -180,7 +181,7 @@ def test_usage_envelope_breakdown_consistent(dtu_id: str) -> None: assert tokens_in >= cache_read + cache_write, ( f"metadata.tokensIn ({tokens_in}) is smaller than cacheReadTokens + cacheWriteTokens " f"({cache_read} + {cache_write} = {cache_read + cache_write}). tokensIn is the CHARGED " - "total, so the derived new-input figure (tokensIn - cacheRead - cacheWrite) would be negative." + "total, so the derived fresh-input figure (tokensIn - cacheRead - cacheWrite) would be negative." ) @@ -212,7 +213,7 @@ def test_usage_envelope_accuracy_vs_raw_events(dtu_id: str) -> None: context = ( f"session={session_id} " f"llm_responses={provider.responses} turn_ids={list(provider.turn_ids)}\n" - f"provider totals: new_input={provider.input_tokens} output={provider.output_tokens} " + f"provider totals: gross_input={provider.input_tokens} output={provider.output_tokens} " f"cache_read={provider.cache_read_tokens} cache_write={provider.cache_write_tokens} " f"charged_input={provider.charged_input}\n" f"envelope metadata: {json.dumps({k: metadata.get(k) for k in sorted(metadata)}, default=str)}" @@ -232,6 +233,7 @@ def test_usage_envelope_accuracy_vs_raw_events(dtu_id: str) -> None: ) assert metadata.get("tokensIn") == provider.charged_input, ( f"metadata.tokensIn ({metadata.get('tokensIn')}) != the provider's CHARGED input total " - f"({provider.charged_input} = new {provider.input_tokens} + cache_read " - f"{provider.cache_read_tokens} + cache_write {provider.cache_write_tokens}).\n{context}" + f"({provider.charged_input} = gross {provider.input_tokens} + cache_write " + f"{provider.cache_write_tokens}; cache_read {provider.cache_read_tokens} is already " + f"inside the gross figure and must NOT be added again).\n{context}" ) diff --git a/tests/e2e/suites/usage/test_usage_wrapper.py b/tests/e2e/suites/usage/test_usage_wrapper.py index 41cb4a7a..e6436821 100644 --- a/tests/e2e/suites/usage/test_usage_wrapper.py +++ b/tests/e2e/suites/usage/test_usage_wrapper.py @@ -210,7 +210,7 @@ def test_usage_result_event_carries_usage(dtu_id: str, wrapper_turn: tuple[str, context = ( f"session={session_id}\n" f"wrapper usage: {json.dumps(usage, sort_keys=True)}\n" - f"provider totals: new_input={provider.input_tokens} output={provider.output_tokens} " + f"provider totals: gross_input={provider.input_tokens} output={provider.output_tokens} " f"cache_read={provider.cache_read_tokens} cache_write={provider.cache_write_tokens} " f"charged_input={provider.charged_input}" ) @@ -222,7 +222,8 @@ def test_usage_result_event_carries_usage(dtu_id: str, wrapper_turn: tuple[str, assert usage.get("input_tokens") == provider.charged_input, ( f"Usage.input_tokens ({usage.get('input_tokens')}) != the provider's CHARGED input " f"total ({provider.charged_input}). Usage.input_tokens mirrors the envelope's " - f"tokensIn, which is new input + cache reads + cache writes.\n{context}" + f"tokensIn, which is gross input + cache writes (cache reads are already " + f"inside the gross figure).\n{context}" ) diff --git a/wrappers/python-py/src/amplifier_agent_py/types.py b/wrappers/python-py/src/amplifier_agent_py/types.py index a936313a..a8882509 100644 --- a/wrappers/python-py/src/amplifier_agent_py/types.py +++ b/wrappers/python-py/src/amplifier_agent_py/types.py @@ -34,16 +34,19 @@ class Usage: ---------- input_tokens: Input tokens **charged**, mirroring the envelope's ``tokensIn``. This - is new input + cache reads + cache writes; the model saw all three as - input and the split is a billing distinction. A host that wants the - new-only figure derives it as + is the provider's gross input plus cache writes. Cache reads are + already counted inside the gross input, so ``cache_read_tokens`` is a + SUBSET of this value, not an addend. A host that wants the fresh-only + figure derives it as ``input_tokens - cache_read_tokens - cache_write_tokens``. output_tokens: Output tokens (envelope ``tokensOut``). cache_read_tokens: - Input tokens served from the provider's prompt cache. + The portion of ``input_tokens`` the provider served from its prompt + cache. Already included above; never add it on top. cache_write_tokens: - Input tokens written into the provider's prompt cache. + Input tokens written into the provider's prompt cache. Billed on top + of the gross input, so this IS a component of ``input_tokens``. cost_usd: Turn cost as a ``Decimal``, parsed from the envelope's decimal ``costUsd`` STRING. Never a float -- binary floats accumulate drift the diff --git a/wrappers/typescript/dist/session.d.ts b/wrappers/typescript/dist/session.d.ts index 5816606f..1dbf8c7e 100644 --- a/wrappers/typescript/dist/session.d.ts +++ b/wrappers/typescript/dist/session.d.ts @@ -62,17 +62,24 @@ export type ChildProcessFactory = (command: string, args: readonly string[], opt */ export interface Usage { /** - * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is new - * input + cache reads + cache writes; the model saw all three as input and - * the split is a billing distinction. A host that wants the new-only figure - * derives it as `inputTokens - cacheReadTokens - cacheWriteTokens`. + * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is the + * provider's gross input plus cache writes. Cache reads are already counted + * inside the gross input, so `cacheReadTokens` is a SUBSET of this value, not + * an addend. A host that wants the fresh-only figure derives it as + * `inputTokens - cacheReadTokens - cacheWriteTokens`. */ inputTokens: number; /** Output tokens (envelope `tokensOut`). */ outputTokens: number; - /** Input tokens served from the provider's prompt cache. */ + /** + * The portion of `inputTokens` the provider served from its prompt cache. + * Already included above; never add it on top. + */ cacheReadTokens: number; - /** Input tokens written into the provider's prompt cache. */ + /** + * Input tokens written into the provider's prompt cache. Billed on top of the + * gross input, so this IS a component of `inputTokens`. + */ cacheWriteTokens: number; /** * Turn cost as a decimal STRING (never a number โ€” see the parity note on diff --git a/wrappers/typescript/src/session.ts b/wrappers/typescript/src/session.ts index 436a2356..9ea34584 100644 --- a/wrappers/typescript/src/session.ts +++ b/wrappers/typescript/src/session.ts @@ -79,17 +79,24 @@ export type ChildProcessFactory = ( */ export interface Usage { /** - * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is new - * input + cache reads + cache writes; the model saw all three as input and - * the split is a billing distinction. A host that wants the new-only figure - * derives it as `inputTokens - cacheReadTokens - cacheWriteTokens`. + * Input tokens **charged**, mirroring the envelope's `tokensIn`. This is the + * provider's gross input plus cache writes. Cache reads are already counted + * inside the gross input, so `cacheReadTokens` is a SUBSET of this value, not + * an addend. A host that wants the fresh-only figure derives it as + * `inputTokens - cacheReadTokens - cacheWriteTokens`. */ inputTokens: number; /** Output tokens (envelope `tokensOut`). */ outputTokens: number; - /** Input tokens served from the provider's prompt cache. */ + /** + * The portion of `inputTokens` the provider served from its prompt cache. + * Already included above; never add it on top. + */ cacheReadTokens: number; - /** Input tokens written into the provider's prompt cache. */ + /** + * Input tokens written into the provider's prompt cache. Billed on top of the + * gross input, so this IS a component of `inputTokens`. + */ cacheWriteTokens: number; /** * Turn cost as a decimal STRING (never a number โ€” see the parity note on diff --git a/wrappers/typescript/test/run-output-parser.test.ts b/wrappers/typescript/test/run-output-parser.test.ts index 04c000ba..f1850ca2 100644 --- a/wrappers/typescript/test/run-output-parser.test.ts +++ b/wrappers/typescript/test/run-output-parser.test.ts @@ -35,7 +35,8 @@ function makeEnvelope( error: null, metadata: { // Protocol 0.4.0 usage block. tokensIn is the CHARGED total: - // 1247 = 900 new + 300 cache reads + 47 cache writes. + // 1247 = 1200 gross input (900 fresh + 300 cache reads, already combined + // by the provider) + 47 cache writes billed on top. tokensIn: 1247, tokensOut: 89, cacheReadTokens: 300, @@ -317,7 +318,8 @@ describe("parseRunOutput โ€” usage block (protocol 0.4.0)", () => { if (ev.type !== "result") throw new Error("expected result event"); // tokensIn is copied straight through as the CHARGED total. The wrapper - // must NOT re-add cache reads/writes: the engine already did. + // must NOT re-add cache writes, and must never add cache reads at all: + // they are already inside the engine's gross input. expect(ev.usage).toEqual({ inputTokens: 1247, outputTokens: 89, From 5c893d35a0454c01ad6ff02c56df5e5901e5b389 Mon Sep 17 00:00:00 2001 From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:00:50 -0400 Subject: [PATCH 3/3] docs(usage): correct the sessionCostTotal rationale after the cost bridge landed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UsageAccumulator's module docstring justified summing per-call cost partly on the grounds that this engine never calls bridge_child_cost, so delegated sub-agent spend could not reach the parent's session.cost channel. It also said to revisit that reasoning if the bridge was ever wired up. spawn.py now calls bridge_child_cost, so the claim is stale. The conclusion is unchanged and no behavior changes: the accumulator sums per-call cost off the display stream and never reads sessionCostTotal, which is the only figure the bridge moves. Two reasons survive and are now the ones stated. sessionCostTotal is session-scoped rather than turn-scoped, so a resumed session carries prior turns into it. And the bridge runs only after a delegation succeeds, so a failed sub-agent's spend never reaches session.cost while its per-call cost events still arrive on the display stream. ๐Ÿค– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../protocol_points/usage_accumulator.py | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/amplifier_agent_lib/protocol_points/usage_accumulator.py b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py index b5a6e522..ea582b5e 100644 --- a/src/amplifier_agent_lib/protocol_points/usage_accumulator.py +++ b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py @@ -40,17 +40,24 @@ ``sessionCostTotal`` (``hook_streaming.on_orchestrator_complete``); a last-event-wins reader reports zero for the whole turn. * ``sessionCostTotal`` is deliberately **not** added to ``cost_usd``, and is not - used in its place either. It is a session-wide total collected from the kernel's + used in its place either. It is a SESSION-wide total collected from the kernel's ``session.cost`` channel, not a per-call cost, so adding it to a sum of per-call - costs double-counts. Substituting it would also LOSE cost: this engine never - calls ``amplifier_foundation.bridge_child_cost`` (see the MVP scope note in - ``spawn.py``), so delegated sub-agent spend never reaches the parent's - ``session.cost`` channel -- while child sessions DO inherit the parent's - ``display.emit`` capability and mount this same streaming hook, so their - per-call ``cost`` events do arrive here. Summing per-call cost off the display - stream is therefore the only path that sees sub-agent spend today. If the cost - bridge is ever wired up, revisit this -- do not switch to ``sessionCostTotal`` - before then. + costs double-counts. + + Substituting it would also under-report, for two reasons that survive the cost + bridge landing in ``spawn.py`` (``bridge_child_cost``, which re-registers a + child's frozen total on the parent coordinator): + + 1. It is session-scoped, not turn-scoped. These totals are per-TURN, and a + resumed session's ``sessionCostTotal`` carries prior turns with it. + 2. The bridge runs only after a delegation SUCCEEDS -- a failed sub-agent's + spend is deliberately never bridged (see the placement note in ``spawn.py``), + so it would be invisible in ``session.cost`` while still being real money. + + Child sessions inherit the parent's ``display.emit`` and mount this same + streaming hook, so their per-call ``cost`` events arrive here regardless of + whether the delegation succeeded. Summing per-call cost off the display stream + therefore stays correct, and stays correct for turns the bridge does not cover. * ``cost`` crosses the wire as a decimal **string** to preserve monetary precision, and is parsed with ``Decimal``. Never float: summing per-call costs as binary floats accumulates drift a host cannot see.