ErrorResponse rejects spec-defined trace_id, so commit rejections are misclassified #102

Summary

ErrorResponse in runcycles/models.py uses extra="forbid" (_SNAKE_CASE_CONFIG) and does not declare trace_id. The server includes trace_id in error bodies, and the protocol spec allows it: ErrorResponse in cycles-protocol-v0.yaml lists a trace_id property. As a result, CyclesResponse.get_error_response() fails validation and returns None for spec-conformant error responses.

Present in v0.5.3 and on main as of 2026-09-27.

Reproduction

from runcycles.models import ErrorResponse
ErrorResponse.model_validate({
    "error": "BUDGET_EXCEEDED", "message": "m", "request_id": "r", "trace_id": "t",
})
# pydantic ValidationError: trace_id — Extra inputs are not permitted

Server 0.1.25.59 response to a commit whose actual exceeds the estimate under overage_policy=REJECT:

HTTP/1.1 409
{"error":"BUDGET_EXCEEDED","message":"Budget exceeded for: Actual exceeds estimate and policy is REJECT","request_id":"...","trace_id":"..."}

Impact

In StreamReservation._handle_commit (runcycles/streaming.py), error_code is derived from get_error_response() and so comes back as None. A recognized rejection therefore does not take the _is_recognized_rejection branch (discard pending, do not release). It falls through to the "unclassifiable client error" branch and is scheduled for retry. Observed log with runcycles 0.5.3 against cycles-server 0.1.25.59:

Stream commit got unclassifiable client error (status=409, error=None); journaling for replay: id=...
Commit retry got non-retryable error: reservation_id=..., status=409, error=BUDGET_EXCEEDED

The final outcome is still correct: nothing is released, the retry engine then classifies BUDGET_EXCEEDED, and the settlement error raised under raise_on_commit_failure=True reports is_budget_exceeded(). But the synchronous classification is wrong, it sends one extra commit request, and the logs are misleading. Every other branch in that method that depends on error_code (LIMIT_EXCEEDED, RESERVATION_EXPIRED, RESERVATION_FINALIZED, IDEMPOTENCY_MISMATCH) is affected the same way whenever the server includes trace_id.

Suggested fix

Add trace_id: str | None = None to ErrorResponse. Also consider whether error envelopes should tolerate unknown fields (extra="allow" or "ignore"), so that future additive spec fields don't break classification. Add a test that parses an error body containing trace_id, and a streaming test asserting that a 409 BUDGET_EXCEEDED commit takes the recognized-rejection branch.

Found while writing the drill for runcycles/cycles-docs#964.