From e36d40e359ccd54ac0b014165af1653e12f6b7e5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 28 Jul 2026 18:14:05 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9A=96=EF=B8=8F=20fix:=20Make=20the=20Commit?= =?UTF-8?q?=20Flag=20the=20Sole=20Billing=20Authority?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-twenty-three review: a committed fill racing a late scope close (user abort or settle timeout during the durable emit) stayed visible — the part is mutated and persisted before the close — yet the deferred accounting's scope gates then skipped the charge: a completed provider call escaping both the label charge and the primary abort accounting. The scope gates on the deferred-usage path are removed; the hook's commit flag is now the single billing authority in BOTH directions. A dropped fill never reaches the accounting callback (billed-never-shown stays impossible), and a committed fill bills regardless of when its scope closed (shown-never-billed now impossible too). The dead `scopeOpen` payload threading is removed with it; the `recordActivityLabelUsage` parameter survives, defaulting open, for callers that own no commit signal. The round's other finding is the twelfth instance of the documented edited+reconnect index limitation, answered on-thread. --- api/server/controllers/agents/client.js | 58 +++++++------------ .../api/src/agents/activityLabels/wiring.ts | 5 +- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 2be9dc1b98..a83ac5ff13 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -397,9 +397,11 @@ class AgentClient extends BaseClient { model, endpointTokenConfig, sameEndpoint, - /** Re-checked immediately before anything is charged or emitted, so a - * scope that closed while this was in flight still suppresses the write. - * Defaults open for callers that own no scope. */ + /** Optional suppression gate, defaulting open. The hook-driven paths + * deliberately pass nothing: they invoke accounting ONLY for a + * COMMITTED fill, and a committed (visible) label must bill even when + * its scope closed during the durable emit — the commit flag, not the + * scope, is the billing authority. */ scopeOpen = () => true, /** The LABEL endpoint's provider — cost math needs it to know whether * cache tokens are folded into `input_tokens` (additive providers like @@ -537,7 +539,6 @@ class AgentClient extends BaseClient { prompt, executingAgentId, deferUsage, - scopeOpen, }) { /** Version gating happens at wiring time via the `sdkCapable` prototype * probe, so this only catches a run that is missing or not yet built. @@ -552,34 +553,22 @@ class AgentClient extends BaseClient { await this.resolveActivityLabelLLM(); const { handleLLMEnd, collected: collectedMetadata } = createMetadataAggregator(); /** - * Gate bound to the OWNING wiring's scope when the caller supplies one. - * The instance-wide fallback ("any scope open") lets a pre-pause - * straggler bill because the RESUMED generation's scope is still open — - * even though its own fill is dropped as closed: billed, never shown. - */ - const scopeStillOpen = - scopeOpen ?? - (() => - (this.activityLabelScopes ?? []).length === 0 || - (this.activityLabelScopes ?? []).some((scope) => scope.closed !== true)); - /** - * Re-read at COMMIT time, not once up front. The settle deadline (or a - * run abort) can close the scope while this accounting is already in - * flight — a single check before the await passes, the charge lands - * after finalization, and the matching `slot.fill` then sees the closed - * scope and drops the label: billed but never surfaced, which is exactly - * what this guard exists to prevent. + * NO scope gate here: the hook invokes this ONLY for a COMMITTED fill, + * and the commit flag is the single billing authority. A scope that + * closes while the fill's durable emit is in flight does not un-commit + * the label — it is persisted and visible — so gating on the scope here + * turned that race into a completed provider call escaping both the + * label charge and the primary abort accounting. The reverse direction + * (billed but never shown) is enforced by the commit gate itself: a + * dropped fill never reaches this callback. */ const recordUsage = async () => { - if (!scopeStillOpen()) { - return; - } await this.recordActivityLabelUsage( collectedMetadata, clientOptions.model, endpointTokenConfig, sameEndpoint, - scopeStillOpen, + undefined, provider, ); }; @@ -840,24 +829,19 @@ class AgentClient extends BaseClient { clientOptions.model, endpointTokenConfig, sameEndpoint, - /** Same commit-time gate as the SDK path: without it, a - * fallback label whose fill was dropped as out-of-scope - * still billed and emitted after finalization. */ - () => labelScope.closed !== true, + /** No scope gate — the hook invokes collect ONLY for a + * COMMITTED fill (the billing authority), and a scope that + * closes during the fill's durable emit must not let a + * visible label escape its charge. Dropped fills never + * reach this callback. */ + undefined, provider, ); }, }; }, ...(sdkCapable && { - /** The wiring's OWN scope, not the instance-wide "any scope open" - * fallback — a pre-pause straggler must not bill just because the - * resumed generation's scope is still open. */ - generateLabel: (payload) => - this.generateActivityLabelViaRun({ - ...payload, - scopeOpen: () => labelScope.closed !== true, - }), + generateLabel: (payload) => this.generateActivityLabelViaRun(payload), }), }); } diff --git a/packages/api/src/agents/activityLabels/wiring.ts b/packages/api/src/agents/activityLabels/wiring.ts index 80144523c7..4605658225 100644 --- a/packages/api/src/agents/activityLabels/wiring.ts +++ b/packages/api/src/agents/activityLabels/wiring.ts @@ -294,7 +294,10 @@ export function createActivityLabelWiring(deps: ActivityLabelHostDeps): { /** Finalization already passed: drop the result rather than * mutating a saved response or emitting into a closed job. * `false` tells the hook the label never surfaced, so its - * usage must not be billed. */ + * usage must not be billed. A scope that closes AFTER this + * check — while the durable emit below is in flight — does + * NOT un-commit: the part is mutated and persisted, so the + * fill still resolves `true` and the committed label bills. */ if (deps.isClosed?.() === true) { return false; }