diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 7d47903f3c..e8e3cc2dc9 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -658,6 +658,12 @@ class AgentClient extends BaseClient { scope.abort.abort(); } }); + /** Settled either way — the abort listener has no remaining work, and + * leaving it attached across HITL approval cycles accumulates dead + * closures on the shared job signal. Idempotent on double settle. */ + for (const scope of this.activityLabelScopes ?? []) { + scope.detach?.(); + } } /** @@ -712,7 +718,10 @@ class AgentClient extends BaseClient { * content itself recorded perfectly well. One retry costs nothing at run * setup and removes the only realistic way the gate goes stale. */ - void GenerationJobManager.markActivityLabels(streamId).catch(() => + /** Retained (not fire-and-forget): label emission is ORDERED after this + * persist, below. The chain settles on failure (warned retry), so a + * lost write can never wedge label emission. */ + const activityLabelsMarked = GenerationJobManager.markActivityLabels(streamId).catch(() => GenerationJobManager.markActivityLabels(streamId).catch(() => { logger.warn( `[AgentClient] Could not flag activity labels for ${streamId}; a label resolving during a resume gap may not be reconciled.`, @@ -756,6 +765,12 @@ class AgentClient extends BaseClient { closeOnAbort(); } else { abortSignal.addEventListener('abort', closeOnAbort, { once: true }); + /** Detached once this segment settles: HITL runs rebuild a wiring + * per approval cycle on the SAME job signal, and `once` only + * removes the listener if an abort actually fires — long + * multi-approval runs would otherwise accumulate obsolete + * closures toward the listener-limit warning. */ + labelScope.detach = () => abortSignal.removeEventListener('abort', closeOnAbort); } } /** Thin wrapper: slot claiming, lane stamping, emit ordering, and settle @@ -770,8 +785,15 @@ class AgentClient extends BaseClient { bumpIndexOffset: () => { this.steerOffsetState.offset += 1; }, - emitLabelEvent: (index, part) => - GenerationJobManager.emitChunk( + emitLabelEvent: async (index, part) => { + /** ORDERED after the flag persist: resume-gap reconciliation is + * gated on the flag, so a label event must never exist before the + * flag does — an immediate cross-replica reconnect could otherwise + * read the job between the two writes, see neither flag nor + * snapshot label, and skip reconciling a label claimed in the + * snapshot→subscribe window. */ + await activityLabelsMarked; + return GenerationJobManager.emitChunk( streamId, { event: ActivityLabelEvents.ON_ACTIVITY_LABEL, @@ -788,7 +810,8 @@ class AgentClient extends BaseClient { * the new response — invisibly, since an empty label renders * nothing — overwriting whatever occupies that slot. */ { durable: true, expectedCreatedAt: this.jobCreatedAt }, - ), + ); + }, trackPendingFill: (fillDone) => { this.pendingActivityLabelFills = this.pendingActivityLabelFills ?? []; this.pendingActivityLabelFills.push(fillDone); diff --git a/packages/api/src/agents/activityLabels/__tests__/host.spec.ts b/packages/api/src/agents/activityLabels/__tests__/host.spec.ts index d201ec2d9f..f6d5d35927 100644 --- a/packages/api/src/agents/activityLabels/__tests__/host.spec.ts +++ b/packages/api/src/agents/activityLabels/__tests__/host.spec.ts @@ -185,12 +185,16 @@ describe('resolveActivityLabelModel model precedence', () => { }); }); - it('strips the primary maxTokens like the title path', async () => { + /** The primary cap is not merely stripped — it is REPLACED with a small + * label-specific one, so a model ignoring the 4–9-word instruction (or + * steered by injected tool output) cannot generate and bill its + * provider-default output for a header. */ + it('replaces the primary maxTokens with the small label cap', async () => { mockGetOptions.mockResolvedValueOnce({ llmConfig: { model: 'resolved', maxTokens: 64_000 }, } as never); const resolved = await resolve({ activityLabel: true, activityModel: 'label-model' }); - expect((resolved.clientOptions as Record).maxTokens).toBeUndefined(); + expect((resolved.clientOptions as Record).maxTokens).toBe(256); }); /** The Anthropic carrier holds client CONSTRUCTION options — for diff --git a/packages/api/src/agents/activityLabels/host.ts b/packages/api/src/agents/activityLabels/host.ts index b62672b93f..92bc5bd096 100644 --- a/packages/api/src/agents/activityLabels/host.ts +++ b/packages/api/src/agents/activityLabels/host.ts @@ -116,6 +116,14 @@ type MaybeAzureConfig = ClientOptions & { configuration?: OpenAIConfiguration; }; +/** Generation cap for label calls — ~25x the largest legitimate 4–9 word + * header, so truncation can never clip a real label. Replaces the stripped + * primary caps: with NO cap, a model that ignores the instruction (or is + * steered by injection in untrusted tool output) generates and BILLS its + * provider-default output on every batch; `normalizeLabelOutput` bounds + * only what persists, not what the provider generates. */ +const LABEL_MAX_OUTPUT_TOKENS = 256; + /** Effective activity-label settings for one endpoint. */ export interface ResolvedActivityConfig { enabled: boolean; @@ -330,6 +338,15 @@ export async function resolveActivityLabelModel({ if (anthropicCarrier != null && clientOptions.clientOptions == null) { clientOptions.clientOptions = anthropicCarrier; } + /** Replace the stripped primary caps with a SMALL one — see + * {@link LABEL_MAX_OUTPUT_TOKENS}. Installed after the filter so the + * omit set cannot remove it; keyed per provider family (Google-style + * wrappers read `maxOutputTokens`, the rest `maxTokens`). */ + if (provider === Providers.GOOGLE || provider === Providers.VERTEXAI) { + (clientOptions as { maxOutputTokens?: number }).maxOutputTokens = LABEL_MAX_OUTPUT_TOKENS; + } else { + (clientOptions as { maxTokens?: number }).maxTokens = LABEL_MAX_OUTPUT_TOKENS; + } if (options.configOptions) { clientOptions.configuration = options.configOptions; }