Never release known spend after execution (#180)

* fix: never release known spend after execution

* ci: retrigger security analysis

Author: amavashev

AUDIT.md

@@ -1,5 +1,15 @@
 # Cycles Protocol v0.1.25 — Client (TypeScript) Audit
 
+**Date:** 2026-08-06 (v0.4.3 — recognized terminal commit rejection no
+longer releases known spend in `withCycles`; post-action failures cannot enter
+the guarded-function release path; actual-evaluation failure commits the
+estimate with an audit marker; and missing required actual configuration fails
+before reservation. Streaming commit rejection keeps the handle finalized so
+broad catch cleanup cannot release it. Lifecycle, post-action, and streaming
+regressions pin these paths. Final verification: 494 tests pass, 6 skip;
+coverage is 95.82% lines and 89.63% branches; lint, typecheck, build, package
+contents, and npm audit are clean.),
+
 **Date:** 2026-07-30 (no runtime change — the durable recovery CI job now
 emits and uploads the profile 0.3 machine-readable evidence report even on
 failure, and the README links its recovery-conformance badge to the public SDK

CHANGELOG.md

@@ -1,5 +1,35 @@
 # Changelog
 
+## [0.4.3] - 2026-08-06
+
+### Fixed
+
+- `withCycles` no longer releases a reservation after a recognized terminal
+  commit rejection. The guarded function has already spent the resource, so
+  returning its reserved budget would undercount known spend.
+- `StreamReservation.commit()` still surfaces a recognized terminal rejection,
+  but keeps the handle finalized. A broad caller catch can no longer turn the
+  failed settlement into a release of known spend.
+- `withCycles` now releases only when the guarded function itself fails.
+  Post-action settlement/setup failures never return budget for work that
+  already ran; a failing or invalid `actual` callback falls back to the
+  validated estimate and records `metadata.actual_source="estimate"`.
+- A configuration that disables estimate fallback without providing `actual`
+  is rejected before a reservation is created or the guarded function runs.
+- `StreamReservation.commit()` now replaces a non-finite, negative, fractional,
+  or unsafe-integer actual with the validated estimate and an
+  `actual_source="estimate"` marker instead of journaling an invalid amount.
+
+### Tests and docs
+
+- Regression tests pin no-release behavior for lifecycle, post-action, and
+  streaming paths.
+- README settlement tables and error guidance now distinguish handler failure
+  (release) from post-action commit rejection (never release).
+- The development lockfile updates `brace-expansion` to 5.0.9, clearing the
+  current high-severity expansion DoS advisories; published runtime
+  dependencies are unchanged.
+
 All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog 1.1.0](https://keepachangelog.com/en/1.1.0/).

README.md

@@ -83,10 +83,12 @@ const result = await callLlm("Hello", 100);
 | Reservation denied | **Neither** | `BudgetExceededError`, `OverdraftLimitExceededError`, or `DebtOutstandingError` thrown; function never executes |
 | `dryRun: true`, any decision | **Neither** | Returns `DryRunResult` or throws; no real reservation created |
 | Function returns successfully | **Commit** | Actual amount charged; unused remainder auto-released |
-| Function throws any error | **Release** | Full reserved amount returned to budget; error re-thrown |
+| Guarded function throws | **Release** | Full reserved amount returned to budget; error re-thrown |
+| Post-action settlement setup throws | **Neither** | Error surfaces, but known spend is never released |
+| `actual` callback throws or returns an invalid amount | **Commit estimate** | Commit carries `metadata.actual_source="estimate"` |
 | Commit fails (5xx / network) | **Retry** | Exponential backoff with configurable attempts |
-| Commit fails (non-retryable 4xx) | **Release** | Reservation released after non-retryable client error |
-| Commit gets RESERVATION_EXPIRED | **Neither** | Server already reclaimed budget on TTL expiry |
+| Commit fails (recognized non-retryable 4xx) | **Neither** | Retry stops and the journal entry is discarded, but known spend is never released |
+| Commit gets RESERVATION_EXPIRED | **Event recovery** | Spend is recorded through `POST /v1/events` because the server already reclaimed the expired reservation |
 | Commit gets RESERVATION_FINALIZED | **Neither** | Already committed or released (idempotent replay) |
 | Commit gets IDEMPOTENCY_MISMATCH | **Neither** | Previous commit already processed; no release attempted |
 
@@ -146,7 +148,8 @@ try {
 ```
 
 The handle is **once-only and race-safe**: in streaming code, multiple terminal paths can fire concurrently (onFinish, error handler, abort signal). Only the first terminal call wins:
-- `commit()` throws `CyclesError` if already finalized (dropping a commit silently hides bugs). If `commit()` fails due to a network or server error, `finalized` resets to `false` so you can retry — but the heartbeat is **not** restarted (restart it manually if needed)
+- `commit()` throws `CyclesError` if already finalized (dropping a commit silently hides bugs). Transient commit failures are durably queued and resolve normally. A recognized terminal rejection throws but leaves `finalized` true, so a broad catch cannot release known spend.
+- An invalid `commit(actual)` amount falls back to the reserved estimate and adds `metadata.actual_source="estimate"`; the handle never journals an invalid amount after the stream ran.
 - `release()` is a silent no-op if already finalized (best-effort by design)
 - `dispose()` stops the heartbeat only, for startup failures before streaming begins
 - `handle.finalized` — check whether the handle has been finalized
@@ -319,7 +322,9 @@ interface WithCyclesConfig {
   estimate: number | ((...args) => number);  // Estimated cost (static or computed from args)
 
   // Actual cost — optional (defaults to estimate if not provided)
-  actual?: number | ((result) => number);    // Actual cost (static or computed from result)
+  actual?: number | ((result) => number);    // Actual cost (static or computed from result).
+                                             // Callback failure/invalid output after the action
+                                             // safely falls back to the estimate.
   useEstimateIfActualNotProvided?: boolean;  // Default: true — use estimate as actual.
                                              // When this fallback is taken the commit carries
                                              // metadata.actual_source = "estimate" so estimated
@@ -472,14 +477,14 @@ try {
 | `DebtOutstandingError` | Outstanding debt blocks new reservations |
 | `ReservationExpiredError` | Operating on an expired reservation |
 | `ReservationFinalizedError` | Operating on an already-committed/released reservation |
-| `TenantClosedError` | The owning tenant is CLOSED (HTTP 409 `TENANT_CLOSED`, runtime spec v0.1.25.13); thrown at reservation time by `withCycles` / `reserveForStream` — commit-time client errors are handled/released internally, and `StreamReservation.commit()` throws generic `CyclesError` |
+| `TenantClosedError` | The owning tenant is CLOSED (HTTP 409 `TENANT_CLOSED`, runtime spec v0.1.25.13); thrown at reservation time by `withCycles` / `reserveForStream` — commit-time client errors never release known spend, and `StreamReservation.commit()` throws generic `CyclesError` for recognized terminal rejection |
 | `CyclesTransportError` | Exported for use in your own code; never thrown by the SDK — transport failures surface as `status === -1` (see below) |
 
 ### Transport failures (status -1)
 
 Transport failures (DNS failure, timeout, connection refused) do not surface as a distinct exception class. The SDK never throws `CyclesTransportError` itself — the class is exported for use in your own code (e.g. wrapping transport-level failures in higher-level integrations). Instead:
 
-- **`withCycles` / `reserveForStream`** throw `CyclesProtocolError` with `status === -1` and `errorCode` `undefined` for **reservation-time** transport failures. Commit-time failures differ: `withCycles` retries the commit in the background (fire-and-forget), while `StreamReservation.commit()` throws and resets `finalized` so you can retry or `release()`.
+- **`withCycles` / `reserveForStream`** throw `CyclesProtocolError` with `status === -1` and `errorCode` `undefined` for **reservation-time** transport failures. Commit-time transient failures are journaled for recovery. `StreamReservation.commit()` throws only for recognized terminal rejection and remains finalized so the caller cannot release known spend.
 - **Programmatic `CyclesClient` calls** never throw on transport failure — they return a `CyclesResponse` with `isTransportError` set and `status` of `-1` (see below).
 
 ### With `CyclesClient` (programmatic)
@@ -725,7 +730,7 @@ A commit records spend that has already happened, so the SDK never lets one exis
 - **Rate limiting:** 429 / `LIMIT_EXCEEDED` is transient everywhere — a rate-limited first commit is scheduled for retry (never released, which would return budget for spend that already happened), and the next attempt waits at least the server's `Retry-After`. The floor is persisted as an absolute timestamp, so a restart mid-wait still honors it.
 - **Authentication failures:** 401/403 on any commit attempt journals the spend (never releases it) and stops the current run's attempts, so spend recorded during a key misconfiguration or rotation window replays once credentials are fixed.
 - **Expired reservations:** a commit answered `RESERVATION_EXPIRED` (or a bodyless HTTP 410) — where the server has already returned the reserved budget to the pool — is recovered via `POST /v1/events` (the protocol's post-hoc direct-debit endpoint), reusing the commit's idempotency key and tagging `metadata.recovered_reservation_id` for reconciliation.
-- **Genuine rejections** — 4xx responses carrying a **recognized** protocol error code (e.g. `UNIT_MISMATCH`) — stop retries and discard the journal entry — retrying cannot fix a malformed commit. Codeless, mangled, or unknown (forward-compat) error codes are *not* treated as rejections: the spend record is journaled and retained for replay instead of being released or discarded.
+- **Genuine rejections** — 4xx responses carrying a **recognized** protocol error code (e.g. `UNIT_MISMATCH`) — stop retries and discard the journal entry because retrying cannot fix a malformed commit. They never release the reservation after the guarded action has spent the resource. Codeless, mangled, or unknown (forward-compat) error codes are *not* treated as rejections: the spend record is journaled and retained for replay instead of being released or discarded.
 - **Delay clamp:** any server-requested wait (`Retry-After`, persisted floors) is honored for at most 1 hour.
 - Applies to `withCycles` **and** the streaming adapter's `handle.commit()`; the programmatic client (`client.commitReservation`) stays manual.
 - Retry timers are ref'd, so a naturally-draining Node process waits for in-flight retries; `process.exit()`, crashes, and signals are covered by journal replay on the next run. Set `journalEnabled: false` (or `CYCLES_JOURNAL_ENABLED=false`) to opt out.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "runcycles",
-  "version": "0.4.2",
+  "version": "0.4.3",
   "description": "TypeScript AI agent runtime control — enforce LLM cost limits, action permissions, and audit trails for agents before execution.",
   "license": "Apache-2.0",
   "author": "runcycles",

package.json

@@ -600,6 +600,22 @@ export class AsyncCyclesLifecycle {
   ): Promise<T> {
     const estimate = evaluateAmount(cfg.estimate, args);
 
+    // Configuration errors must fail before a reservation exists. Otherwise a
+    // successful guarded action could be followed by a missing-actual error and
+    // the generic failure path would incorrectly return budget for known spend.
+    if (
+      !cfg.dryRun &&
+      cfg.actual === undefined &&
+      cfg.useEstimateIfActualNotProvided === false
+    ) {
+      throw new Error(
+        "actual expression is required when useEstimateIfActualNotProvided is false",
+      );
+    }
+    if (!cfg.dryRun && typeof cfg.actual === "number") {
+      validateNonNegative(cfg.actual, "actual");
+    }
+
     const createBody = buildReservationBody(
       cfg,
       estimate,
@@ -677,18 +693,36 @@ export class AsyncCyclesLifecycle {
       ctx,
     );
 
+    let guardedFunctionCompleted = false;
     try {
       const result = await runWithContext(ctx, () => fn(...args));
+      guardedFunctionCompleted = true;
       const methodElapsed = Math.round(performance.now() - resT2);
 
       // Resolve actual
       const useEstimateFallback = cfg.useEstimateIfActualNotProvided !== false;
-      const { amount: actualAmount, usedEstimateFallback } = evaluateActual(
-        cfg.actual,
-        result,
-        estimate,
-        useEstimateFallback,
-      );
+      let actualAmount: number;
+      let usedEstimateFallback: boolean;
+      try {
+        const resolved = evaluateActual(
+          cfg.actual,
+          result,
+          estimate,
+          useEstimateFallback,
+        );
+        validateNonNegative(resolved.amount, "actual");
+        ({ amount: actualAmount, usedEstimateFallback } = resolved);
+      } catch (err) {
+        // The action already ran. Preserve the spend by committing the validated
+        // estimate instead of releasing or losing the reservation because a
+        // user-supplied post-action accounting callback failed.
+        actualAmount = estimate;
+        usedEstimateFallback = true;
+        console.warn(
+          `[runcycles] Actual evaluation failed after the guarded action completed; committing the estimate instead: ${reservationId}`,
+          err,
+        );
+      }
 
       // Build commit
       let metrics = ctx.metrics;
@@ -727,7 +761,14 @@ export class AsyncCyclesLifecycle {
 
       return result;
     } catch (err) {
-      await this._handleRelease(reservationId, "guarded_method_failed");
+      if (!guardedFunctionCompleted) {
+        await this._handleRelease(reservationId, "guarded_method_failed");
+      } else {
+        console.error(
+          `[runcycles] Post-action settlement failed; not releasing known spend: ${reservationId}`,
+          err,
+        );
+      }
       throw err;
     } finally {
       if (heartbeatRef) {
@@ -827,12 +868,12 @@ export class AsyncCyclesLifecycle {
           parsedErrorCode !== ErrorCode.UNKNOWN &&
           !isRetryableErrorCode(parsedErrorCode)
         ) {
-          // Recognized protocol code — a genuine rejection the retry
-          // engine cannot fix. Releasing returns the reserved budget.
+          // Recognized protocol code — a genuine rejection the retry engine
+          // cannot fix. The guarded function has already spent the resource,
+          // so releasing here would return budget for known spend.
           this._retryEngine.discardPending(reservationId);
-          await this._handleRelease(
-            reservationId,
-            `commit_rejected_${errorCode}`,
+          console.error(
+            `[runcycles] Commit was rejected after spend was recorded locally (error=${String(errorCode)}); not releasing known spend: ${reservationId}`,
           );
           return;
         }

src/lifecycle.ts

@@ -99,8 +99,9 @@ export interface StreamReservation {
    * (codeless or unrecognized error codes) are handled internally —
    * journaled and retried in the background (with a `POST /v1/events`
    * fallback once the reservation has expired) — and resolve normally.
-   * Only genuine rejections carrying a recognized protocol error code
-   * (e.g. UNIT_MISMATCH) reset `finalized` and throw.
+   * Genuine rejections carrying a recognized protocol error code (e.g.
+   * UNIT_MISMATCH) discard the unrecoverable journal entry and throw, but
+   * remain finalized so a broad catch cannot release known spend.
    */
   commit(
     actual: number,
@@ -732,17 +733,32 @@ export async function reserveForStream(
       if (finalized) {
         throw new CyclesError("StreamReservation already finalized");
       }
+      let settledActual = actual;
+      let settledMetadata = metadata;
+      try {
+        validateNonNegative(actual, "actual");
+      } catch (err) {
+        // The stream has already consumed resources. An invalid caller-side
+        // measurement must not strand or release the reservation; settle the
+        // validated estimate and mark the evidence instead.
+        settledActual = estimate;
+        settledMetadata = { ...(metadata ?? {}), actual_source: "estimate" };
+        console.warn(
+          `[runcycles] Stream actual is invalid; committing the estimate instead: ${reservationId}`,
+          err,
+        );
+      }
       finalized = true;
       stopHeartbeat();
       const commitBody: Record<string, unknown> = {
         idempotency_key: randomUUID(),
-        actual: { unit, amount: actual },
+        actual: { unit, amount: settledActual },
       };
       if (metrics && !isMetricsEmpty(metrics)) {
         commitBody.metrics = metricsToWire(metrics);
       }
-      if (metadata) {
-        commitBody.metadata = metadata;
+      if (settledMetadata) {
+        commitBody.metadata = settledMetadata;
       }
       const eventFallback = buildEventFallbackBody(
         reservationId,
@@ -830,14 +846,10 @@ export async function reserveForStream(
         !isRetryableErrorCode(parsedErrorCode)
       ) {
         // Genuine rejection (e.g. UNIT_MISMATCH — a recognized protocol
-        // code): reset finalized so the caller can correct and retry
-        // commit or fall back to release. The heartbeat is NOT restarted
-        // to avoid spawning duplicate heartbeat chains (an old in-flight
-        // extend's .finally→tick could race with a new startHeartbeat
-        // call). The reservation's remaining TTL should give the caller
-        // enough time to retry or release.
+        // code): the retry engine cannot fix the malformed commit. The spend
+        // already happened, so keep the handle finalized; a broad caller
+        // catch must not be able to release the reserved budget.
         retryEngine.discardPending(reservationId);
-        finalized = false;
         throw new CyclesError(
           `Commit failed with status ${response.status}: ${response.errorMessage ?? "unknown error"}`,
         );

src/streaming.ts

@@ -20,8 +20,8 @@ export function validateSubject(subject: Subject | undefined): void {
 }
 
 export function validateNonNegative(value: number, name: string): void {
-  if (value < 0) {
-    throw new Error(`${name} must be non-negative, got ${value}`);
+  if (!Number.isSafeInteger(value) || value < 0) {
+    throw new Error(`${name} must be a finite, non-negative safe integer, got ${value}`);
   }
 }