diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c9e061a..dc937381 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,65 @@ 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: 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. + + 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..cd9df412 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,21 @@ 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: 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, + 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 +76,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 +88,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 +106,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 +121,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 +130,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 +274,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..35989929 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 (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 +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..b48d6d0b 100644 --- a/src/amplifier_agent_lib/protocol/methods.py +++ b/src/amplifier_agent_lib/protocol/methods.py @@ -8,9 +8,22 @@ 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 (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). @@ -109,11 +122,40 @@ 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: 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 already counted in gross input that the provider + #: served from its prompt cache. Reported for visibility; never added on top. + cacheReadTokens: int + #: 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 + #: 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..ea582b5e --- /dev/null +++ b/src/amplifier_agent_lib/protocol_points/usage_accumulator.py @@ -0,0 +1,215 @@ +"""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: + +* ``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``, 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 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. +""" + +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 + ---------- + 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`` -- the cached portion already counted inside + ``gross_input``. Reported for visibility; never added to it. + 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.gross_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.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")) + + # 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: gross input + cache writes. + + ``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.gross_input + 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.gross_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..6df6d9df --- /dev/null +++ b/tests/e2e/suites/usage/events_oracle.py @@ -0,0 +1,190 @@ +"""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``. 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 +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 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 + 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: 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: + """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..9048081c --- /dev/null +++ b/tests/e2e/suites/usage/test_usage_envelope.py @@ -0,0 +1,239 @@ +"""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 = gross input + cache writes + tokensOut int output tokens + 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 + +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 parts cannot exceed it. + + ``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"), [])) + + 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 fresh-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: 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)}" + ) + + 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} = 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 new file mode 100644 index 00000000..e6436821 --- /dev/null +++ b/tests/e2e/suites/usage/test_usage_wrapper.py @@ -0,0 +1,395 @@ +"""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: 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}" + ) + + 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 gross input + cache writes (cache reads are already " + f"inside the gross figure).\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..a8882509 100644 --- a/wrappers/python-py/src/amplifier_agent_py/types.py +++ b/wrappers/python-py/src/amplifier_agent_py/types.py @@ -11,10 +11,56 @@ 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 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: + 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. 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 + 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 +83,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 +144,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..1dbf8c7e 100644 --- a/wrappers/typescript/dist/session.d.ts +++ b/wrappers/typescript/dist/session.d.ts @@ -40,23 +40,117 @@ 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 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; + /** + * 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. 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 + * 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 +159,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 +277,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..9ea34584 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,114 @@ 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 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; + /** + * 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. 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 + * 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 +174,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 +307,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 +387,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 +616,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 +628,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 +652,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 +662,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..f1850ca2 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,24 @@ 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 = 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, + 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 +64,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 +82,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 +101,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 +128,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 +157,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 +180,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 +199,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 +218,279 @@ 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 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, + 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"); }); });