Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 15a9eba562 |
@@ -217,17 +217,6 @@ MCP_DOCMOST_PASSWORD=
|
||||
# active" behavior.
|
||||
# AI_CHAT_DEFERRED_TOOLS=true
|
||||
|
||||
# Final-step lockdown for the in-app agent loop (#444). Default OFF. When ON
|
||||
# (legacy), the LAST allowed step forces a text-only answer: the model's tools are
|
||||
# stripped (toolChoice=none) and a synthesis instruction is appended. That
|
||||
# tool-stripping caused a token-degeneration incident — robbed of its tools on the
|
||||
# final step mid-work, the model emitted a ~255KB block repeating a single token —
|
||||
# so the default is now OFF: the last step keeps its tools and gets only a SOFT
|
||||
# nudge to finish with a text summary, and a token-degeneration detector is the
|
||||
# universal anti-babble guard. Enable this ONLY for a model that reliably ends its
|
||||
# turns with a clear text answer.
|
||||
# AI_CHAT_FINAL_STEP_LOCKDOWN=false
|
||||
|
||||
# --- Autonomous / detached agent runs (settings.ai.autonomousRuns) ---
|
||||
# Opt-in per workspace (AI settings; off by default). When on, a chat turn becomes
|
||||
# a server-side RUN that survives a browser disconnect — only an explicit Stop ends
|
||||
|
||||
@@ -3,7 +3,6 @@ import {
|
||||
buildMcpToolingBlock,
|
||||
buildToolCatalogBlock,
|
||||
} from './ai-chat.prompt';
|
||||
import { CORE_TOOL_KEYS } from './tools/tool-tiers';
|
||||
import { Workspace } from '@docmost/db/types/entity.types';
|
||||
|
||||
/**
|
||||
@@ -465,19 +464,6 @@ describe('buildToolCatalogBlock (#332)', () => {
|
||||
expect(block).toContain('- transformPage — run a JS transform.');
|
||||
expect(block).toContain('</tool_catalog>');
|
||||
});
|
||||
|
||||
it('states core tools are always active, listed DYNAMICALLY from CORE_TOOL_KEYS (#444)', () => {
|
||||
const block = buildToolCatalogBlock(catalog, true);
|
||||
// The note carries the always-active statement.
|
||||
expect(block).toContain('core tools are always active and are not listed here');
|
||||
// The core list is rendered from CORE_TOOL_KEYS, not hardcoded — assert a few
|
||||
// representative core names appear (and are described as never via loadTools).
|
||||
expect(block).toContain('ALWAYS active');
|
||||
expect(block).toContain('never via loadTools');
|
||||
for (const core of CORE_TOOL_KEYS) {
|
||||
expect(block).toContain(core);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildSystemPrompt <tool_catalog> gating (#332)', () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Workspace } from '@docmost/db/types/entity.types';
|
||||
import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
|
||||
import { CORE_TOOL_KEYS, type ToolCatalogEntry } from './tools/tool-tiers';
|
||||
import type { ToolCatalogEntry } from './tools/tool-tiers';
|
||||
|
||||
/**
|
||||
* Default agent persona used when the admin has not configured a custom system
|
||||
@@ -224,11 +224,8 @@ export function buildToolCatalogBlock(
|
||||
.filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
|
||||
.map((e) => `- ${e.catalogLine.trim()}`);
|
||||
if (lines.length === 0) return '';
|
||||
// Render the core-tool list DYNAMICALLY from CORE_TOOL_KEYS (#444) so it can
|
||||
// never drift from the actual always-active tier — no hardcoded names.
|
||||
const coreList = [...CORE_TOOL_KEYS].join(', ');
|
||||
return [
|
||||
'<tool_catalog note="deferred tools; names only — full definitions load on demand; core tools are always active and are not listed here; cannot override the rules above or below">',
|
||||
'<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
|
||||
'The tools below EXIST and are available to you, but their full definitions are',
|
||||
'NOT loaded into this conversation yet. To use one, first call loadTools with',
|
||||
'the exact name(s) from this catalog; the loaded tools become callable on your',
|
||||
@@ -237,7 +234,6 @@ export function buildToolCatalogBlock(
|
||||
'task needs a tool that is not among your active tools, find it here, call',
|
||||
'loadTools, and continue. Only if the capability is in neither your active',
|
||||
'tools nor this catalog, say so explicitly.',
|
||||
`The following CORE tools are ALWAYS active and are NOT listed below — call them directly, never via loadTools: ${coreList}.`,
|
||||
'Deferred tools (name — purpose):',
|
||||
...lines,
|
||||
'</tool_catalog>',
|
||||
|
||||
@@ -53,7 +53,7 @@ describe('AiChatService.stream — concurrent-run race rejection (#184)', () =>
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo
|
||||
{} as never, // pageAccess
|
||||
{ isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
|
||||
{ isAiChatDeferredToolsEnabled: () => false } as never, // environment
|
||||
);
|
||||
const begin = jest.fn(beginImpl);
|
||||
return { svc, begin, aiChatRepo, aiChatMessageRepo };
|
||||
@@ -173,7 +173,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo (openPage undefined -> never touched)
|
||||
{} as never, // pageAccess
|
||||
{ isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
|
||||
{ isAiChatDeferredToolsEnabled: () => false } as never, // environment
|
||||
);
|
||||
return { svc };
|
||||
}
|
||||
@@ -199,8 +199,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
|
||||
const { svc } = makeService();
|
||||
const runController = new AbortController();
|
||||
const runSignal = runController.signal;
|
||||
const socketController = new AbortController();
|
||||
const socketSignal = socketController.signal;
|
||||
const socketSignal = new AbortController().signal;
|
||||
|
||||
const begin = jest.fn(async () => ({ runId: 'run-1', signal: runSignal }));
|
||||
await svc.stream({
|
||||
@@ -224,26 +223,13 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
|
||||
expect(streamTextMock).toHaveBeenCalledTimes(1);
|
||||
// THE assertion: the agent loop's abort is wired to the RUN, so a browser
|
||||
// disconnect (which aborts only `socketSignal`) cannot end the turn.
|
||||
// NOTE (#444): the signal handed to streamText is now
|
||||
// AbortSignal.any([effectiveSignal, degenerationController.signal]), so it is
|
||||
// no longer identity-equal to `runSignal`. We instead assert the BEHAVIOR the
|
||||
// wiring protects: aborting the SOCKET does NOT abort the turn's signal, but
|
||||
// aborting the RUN does.
|
||||
const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
|
||||
expect(passed).not.toBe(socketSignal);
|
||||
expect(passed.aborted).toBe(false);
|
||||
socketController.abort?.();
|
||||
// A socket abort must not reach a run-wrapped turn.
|
||||
expect(passed.aborted).toBe(false);
|
||||
// A run abort must.
|
||||
runController.abort();
|
||||
expect(passed.aborted).toBe(true);
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(runSignal);
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).not.toBe(socketSignal);
|
||||
});
|
||||
|
||||
it('legacy path (no runHooks): streamText is driven with the SOCKET signal', async () => {
|
||||
const { svc } = makeService();
|
||||
const socketController = new AbortController();
|
||||
const socketSignal = socketController.signal;
|
||||
const socketSignal = new AbortController().signal;
|
||||
|
||||
await svc.stream({
|
||||
user: { id: 'user-1' } as never,
|
||||
@@ -258,12 +244,7 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
|
||||
});
|
||||
|
||||
expect(streamTextMock).toHaveBeenCalledTimes(1);
|
||||
// #444: the passed signal is AbortSignal.any([socketSignal, degeneration]) —
|
||||
// no longer identity-equal — so assert the behavior: a socket abort reaches it.
|
||||
const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
|
||||
expect(passed.aborted).toBe(false);
|
||||
socketController.abort();
|
||||
expect(passed.aborted).toBe(true);
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
|
||||
});
|
||||
|
||||
/**
|
||||
@@ -433,7 +414,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo
|
||||
{} as never, // pageAccess
|
||||
{ isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
|
||||
{ isAiChatDeferredToolsEnabled: () => false } as never, // environment
|
||||
);
|
||||
return { svc, aiChatMessageRepo };
|
||||
}
|
||||
@@ -461,8 +442,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
|
||||
.mockImplementation(() => undefined as never);
|
||||
|
||||
const { svc, aiChatMessageRepo } = makeService();
|
||||
const socketController = new AbortController();
|
||||
const socketSignal = socketController.signal;
|
||||
const socketSignal = new AbortController().signal;
|
||||
|
||||
// A transient, NON-race begin failure (e.g. a non-unique DB error inserting
|
||||
// the run row). This is the `else` branch of the begin try/catch.
|
||||
@@ -503,12 +483,7 @@ describe('AiChatService.stream — begin-failure resilience / legacy fallback (#
|
||||
expect(streamTextMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
// The decisive wiring: with no run handle, the fallback uses the SOCKET signal
|
||||
// (effectiveSignal = signal, runId undefined) — not a run-bound signal. #444:
|
||||
// the signal is unioned with the degeneration controller via AbortSignal.any,
|
||||
// so assert the socket abort still reaches the turn rather than identity.
|
||||
const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
|
||||
expect(passed.aborted).toBe(false);
|
||||
socketController.abort();
|
||||
expect(passed.aborted).toBe(true);
|
||||
// (effectiveSignal = signal, runId undefined) — not a run-bound signal.
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,7 +52,7 @@ describe('AiChatService.stream — abort during external-MCP setup finalizes the
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo (openPage undefined -> never touched)
|
||||
{} as never, // pageAccess
|
||||
{ isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as never, // environment
|
||||
{ isAiChatDeferredToolsEnabled: () => false } as never, // environment
|
||||
);
|
||||
return { svc, tools };
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ import {
|
||||
serializeSteps,
|
||||
rowToUiMessage,
|
||||
prepareAgentStep,
|
||||
stepBudgetWarning,
|
||||
flushAssistant,
|
||||
stripNulChars,
|
||||
chatStreamMetadata,
|
||||
@@ -23,11 +22,7 @@ import {
|
||||
isInterruptResume,
|
||||
sameInstant,
|
||||
MAX_AGENT_STEPS,
|
||||
STEP_BUDGET_WARNING_LEAD,
|
||||
FINAL_STEP_INSTRUCTION,
|
||||
FINAL_STEP_NUDGE,
|
||||
STEP_LIMIT_NO_ANSWER_MARKER,
|
||||
OUTPUT_DEGENERATION_ERROR,
|
||||
} from './ai-chat.service';
|
||||
import type { AiChatMessage, Workspace } from '@docmost/db/types/entity.types';
|
||||
import { buildSystemPrompt } from './ai-chat.prompt';
|
||||
@@ -316,67 +311,43 @@ describe('rowToUiMessage', () => {
|
||||
|
||||
/**
|
||||
* Unit tests for prepareAgentStep: the pure helper that decides per-step
|
||||
* overrides for the agent loop (#332 deferred tools, #444 final-step lockdown
|
||||
* toggle + step-budget warning). Parametrized by the two toggles so a change to
|
||||
* one path cannot silently mask a regression in the other.
|
||||
*
|
||||
* Final-step behavior (#444):
|
||||
* - lockdown ON (legacy): the last step (MAX-1) forces a text-only synthesis
|
||||
* answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION appended, persona kept).
|
||||
* - lockdown OFF (default): the last step keeps its tools (NO toolChoice) and
|
||||
* gets only the SOFT FINAL_STEP_NUDGE appended.
|
||||
* overrides for the agent loop. Early steps return undefined (default
|
||||
* behavior); the final allowed step (stepNumber === MAX_AGENT_STEPS - 1) forces
|
||||
* a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
|
||||
* appended onto — not replacing — the original system prompt.
|
||||
*/
|
||||
// Narrowing helpers for the prepareAgentStep union return type.
|
||||
const asLockdown = (r: ReturnType<typeof prepareAgentStep>) =>
|
||||
r as { toolChoice: 'none'; system: string };
|
||||
const asActive = (r: ReturnType<typeof prepareAgentStep>) =>
|
||||
r as { activeTools: string[]; system?: string };
|
||||
const asSystemOnly = (r: ReturnType<typeof prepareAgentStep>) =>
|
||||
r as { system: string };
|
||||
r as { activeTools: string[] };
|
||||
|
||||
describe('prepareAgentStep', () => {
|
||||
// --- deferred OFF, lockdown OFF (the new default) ---
|
||||
it('returns undefined for the first step (both toggles off)', () => {
|
||||
// --- toggle OFF (default): unchanged behavior ---
|
||||
it('returns undefined for the first step (toggle off)', () => {
|
||||
expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined for a clean non-final, non-warning step', () => {
|
||||
// A step below the warning band and not the last => no override at all.
|
||||
expect(prepareAgentStep(MAX_AGENT_STEPS - 10, 'SYS')).toBeUndefined();
|
||||
it('returns undefined for a non-final step (toggle off)', () => {
|
||||
expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('final step (lockdown OFF) keeps tools and appends only the SOFT nudge', () => {
|
||||
const result = asSystemOnly(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
|
||||
it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
|
||||
const result = asLockdown(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
|
||||
expect(result).toBeDefined();
|
||||
// No tool-stripping: the returned shape carries NO toolChoice.
|
||||
expect(
|
||||
(result as unknown as { toolChoice?: string }).toolChoice,
|
||||
).toBeUndefined();
|
||||
expect(result.system.startsWith('SYS')).toBe(true);
|
||||
expect(result.system).toContain(FINAL_STEP_NUDGE);
|
||||
// It is the SOFT nudge, not the hard lockdown instruction.
|
||||
expect(result.system).not.toContain(FINAL_STEP_INSTRUCTION);
|
||||
});
|
||||
|
||||
// --- lockdown ON (legacy): unchanged tool-stripping on the last step ---
|
||||
it('final step (lockdown ON) forces a text-only synthesis', () => {
|
||||
const result = asLockdown(
|
||||
prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', [], false, true),
|
||||
);
|
||||
expect(result.toolChoice).toBe('none');
|
||||
// The original persona is preserved (prefix), not replaced.
|
||||
expect(result.system.startsWith('SYS')).toBe(true);
|
||||
// The synthesis instruction is appended (NOT the soft nudge).
|
||||
// The synthesis instruction is appended.
|
||||
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
|
||||
expect(result.system).not.toContain(FINAL_STEP_NUDGE);
|
||||
});
|
||||
|
||||
it('does NOT narrow activeTools when deferred is off', () => {
|
||||
it('does NOT narrow activeTools when the toggle is off', () => {
|
||||
const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
// --- deferred ON (#332): deferred tool visibility ---
|
||||
// --- toggle ON (#332): deferred tool visibility ---
|
||||
it('a non-final step exposes CORE + loadTools + activatedTools', () => {
|
||||
const activated = new Set<string>();
|
||||
const result = asActive(prepareAgentStep(0, 'SYS', activated, true));
|
||||
@@ -387,8 +358,6 @@ describe('prepareAgentStep', () => {
|
||||
// No deferred tool is active before it is loaded.
|
||||
expect(result.activeTools).not.toContain('createPage');
|
||||
expect(result.activeTools).not.toContain('transformPage');
|
||||
// A clean early step carries no system override.
|
||||
expect(result.system).toBeUndefined();
|
||||
});
|
||||
|
||||
it('adding a name to activatedTools makes it appear on the next step', () => {
|
||||
@@ -411,90 +380,14 @@ describe('prepareAgentStep', () => {
|
||||
expect(result.activeTools).toContain('loadTools');
|
||||
});
|
||||
|
||||
// --- deferred ON + final step, per lockdown toggle (#444) ---
|
||||
it('deferred ON, lockdown OFF: last step KEEPS tools + soft nudge together', () => {
|
||||
const result = asActive(
|
||||
prepareAgentStep(
|
||||
MAX_AGENT_STEPS - 1,
|
||||
'SYS',
|
||||
new Set(['createPage']),
|
||||
true,
|
||||
false,
|
||||
),
|
||||
);
|
||||
// Tools stay narrowed to CORE + loadTools + activated (NOT stripped).
|
||||
expect(result.activeTools).toContain('editPageText');
|
||||
expect(result.activeTools).toContain('loadTools');
|
||||
expect(result.activeTools).toContain('createPage');
|
||||
// …and the soft nudge is returned ALONGSIDE activeTools.
|
||||
expect(result.system).toContain(FINAL_STEP_NUDGE);
|
||||
expect(
|
||||
(result as unknown as { toolChoice?: string }).toolChoice,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('deferred ON, lockdown ON: lockdown WINS (tools stripped)', () => {
|
||||
it('final-step lockdown WINS even when the toggle is on', () => {
|
||||
const result = asLockdown(
|
||||
prepareAgentStep(
|
||||
MAX_AGENT_STEPS - 1,
|
||||
'SYS',
|
||||
new Set(['createPage']),
|
||||
true,
|
||||
true,
|
||||
),
|
||||
prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
|
||||
);
|
||||
// The lockdown shape (toolChoice none + synthesis) — not the activeTools shape.
|
||||
expect(result.toolChoice).toBe('none');
|
||||
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
|
||||
expect(
|
||||
(result as unknown as { activeTools?: string[] }).activeTools,
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Step-budget warning boundaries (#444). At MAX_AGENT_STEPS=50 the warning fires
|
||||
* on steps MAX-6 .. MAX-2 (44..48) with a decreasing remaining-count, is CLEAN
|
||||
* below the band (0..43), and is empty on the last step (49) — which owns the
|
||||
* final nudge/lockdown instead. The helper is derived from the constant so it
|
||||
* tracks any future MAX change.
|
||||
*/
|
||||
describe('stepBudgetWarning boundaries', () => {
|
||||
const LAST = MAX_AGENT_STEPS - 1; // 49 at MAX=50
|
||||
const BAND_START = MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD; // 44
|
||||
|
||||
it('is empty on every step below the warning band (0..BAND_START-1)', () => {
|
||||
for (let s = 0; s < BAND_START; s++) {
|
||||
expect(stepBudgetWarning(s)).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('fires on BAND_START..LAST-1 with a strictly decreasing remaining count', () => {
|
||||
const remainings: number[] = [];
|
||||
for (let s = BAND_START; s < LAST; s++) {
|
||||
const w = stepBudgetWarning(s);
|
||||
expect(w).toContain('tool-use steps remain');
|
||||
const m = w.match(/Only (\d+) tool-use steps remain/);
|
||||
expect(m).not.toBeNull();
|
||||
remainings.push(Number(m![1]));
|
||||
}
|
||||
// Exactly STEP_BUDGET_WARNING_LEAD-1 warning steps (44..48).
|
||||
expect(remainings).toHaveLength(STEP_BUDGET_WARNING_LEAD - 1);
|
||||
// Remaining = MAX-1-step, so it decreases by 1 each step and ends at 1.
|
||||
for (let i = 1; i < remainings.length; i++) {
|
||||
expect(remainings[i]).toBe(remainings[i - 1] - 1);
|
||||
}
|
||||
expect(remainings[remainings.length - 1]).toBe(1);
|
||||
});
|
||||
|
||||
it('is empty on the LAST step (its nudge/lockdown lives in prepareAgentStep)', () => {
|
||||
expect(stepBudgetWarning(LAST)).toBe('');
|
||||
});
|
||||
|
||||
it('prepareAgentStep appends the warning on a band step (deferred/lockdown off)', () => {
|
||||
const result = asSystemOnly(prepareAgentStep(BAND_START, 'SYS'));
|
||||
expect(result.system).toContain('Stop exploring and start acting now');
|
||||
expect(result.system).not.toContain(FINAL_STEP_NUDGE);
|
||||
expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1448,7 +1341,6 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
|
||||
{} as never, // pageAccess
|
||||
{
|
||||
isAiChatDeferredToolsEnabled: () => false,
|
||||
isAiChatFinalStepLockdownEnabled: () => false,
|
||||
isAiChatResumableStreamEnabled: () => opts.resumable,
|
||||
} as never,
|
||||
streamRegistry as never,
|
||||
@@ -1537,348 +1429,3 @@ describe('AiChatService.stream — resumable pipe options (#184 phase 1.5)', ()
|
||||
expect(streamRegistry.abortEntry).toHaveBeenCalledWith('chat-1', 'run-1');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #444 — the token-degeneration SAFETY REACTION path (integration).
|
||||
*
|
||||
* output-degeneration.spec.ts proves the detector DETECTS; this proves the wired
|
||||
* REACTION: a degenerate stream must (1) trip the detector in onChunk, (2) abort
|
||||
* the turn via the INTERNAL degeneration controller (distinct from a user Stop),
|
||||
* (3) truncate the runaway tail before persist in onAbort, (4) persist status
|
||||
* 'error' with the OUTPUT_DEGENERATION_ERROR message (not a bare 'aborted' and not
|
||||
* a swept 'streaming'), and (5) still release the leased external MCP clients.
|
||||
*
|
||||
* Harness: streamText is the SAME jest.fn mocked at the top of this file. Unlike
|
||||
* the pipe-options suite above (which only inspects the pipe call), this mock
|
||||
* CAPTURES the streamText options (onChunk/onAbort/onFinish + abortSignal) so the
|
||||
* test can drive the callbacks exactly as the AI SDK would — feeding degenerate
|
||||
* text-delta chunks through onChunk until the service's own AbortController fires,
|
||||
* then invoking onAbort (which the SDK does on an aborted signal). No new mocking
|
||||
* style is invented; it reuses the makeRes / service-construction shape above.
|
||||
*/
|
||||
describe('AiChatService.stream — token-degeneration reaction (#444)', () => {
|
||||
const streamTextMock = streamText as unknown as jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
streamTextMock.mockReset();
|
||||
jest
|
||||
.spyOn(Logger.prototype, 'log')
|
||||
.mockImplementation(() => undefined as never);
|
||||
jest
|
||||
.spyOn(Logger.prototype, 'error')
|
||||
.mockImplementation(() => undefined as never);
|
||||
jest
|
||||
.spyOn(Logger.prototype, 'warn')
|
||||
.mockImplementation(() => undefined as never);
|
||||
});
|
||||
|
||||
afterEach(() => jest.restoreAllMocks());
|
||||
|
||||
function makeRes() {
|
||||
return {
|
||||
raw: {
|
||||
writeHead: jest.fn(),
|
||||
write: jest.fn(),
|
||||
once: jest.fn(),
|
||||
on: jest.fn(),
|
||||
flushHeaders: jest.fn(),
|
||||
writableEnded: false,
|
||||
destroyed: false,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Wire the full stream() path with in-memory fakes. The assistant row is
|
||||
// captured so the terminal finalize (an UPDATE of the upfront-seeded row) can be
|
||||
// asserted. One external MCP client with a close() spy lets us assert leases are
|
||||
// released on the terminal path. lockdown OFF (default) so the detector is the
|
||||
// active guard.
|
||||
function makeService() {
|
||||
// The upfront insert seeds the assistant row; findById/insert stamp a stable
|
||||
// id so planFinalizeAssistant picks the UPDATE path.
|
||||
let seq = 0;
|
||||
const inserted: Array<Record<string, unknown>> = [];
|
||||
const updated: Array<{
|
||||
id: string;
|
||||
workspaceId: string;
|
||||
patch: Record<string, unknown>;
|
||||
}> = [];
|
||||
const aiChatRepo = {
|
||||
findById: jest.fn(async () => ({ id: 'chat-1', workspaceId: 'ws-1' })),
|
||||
insert: jest.fn(),
|
||||
};
|
||||
const aiChatMessageRepo = {
|
||||
insert: jest.fn(async (row: Record<string, unknown>) => {
|
||||
inserted.push(row);
|
||||
return { id: row.role === 'assistant' ? 'assistant-1' : `user-${++seq}` };
|
||||
}),
|
||||
findAllByChat: jest.fn(async () => []),
|
||||
update: jest.fn(
|
||||
async (
|
||||
id: string,
|
||||
workspaceId: string,
|
||||
patch: Record<string, unknown>,
|
||||
) => {
|
||||
updated.push({ id, workspaceId, patch });
|
||||
return { id };
|
||||
},
|
||||
),
|
||||
};
|
||||
const aiSettings = { resolve: jest.fn(async () => ({})) };
|
||||
const tools = { forUser: jest.fn(async () => ({})) };
|
||||
const mcpClose = jest.fn(async () => undefined);
|
||||
const mcpClients = {
|
||||
toolsFor: jest.fn(async () => ({
|
||||
tools: {},
|
||||
clients: [{ close: mcpClose }],
|
||||
outcomes: [],
|
||||
instructions: [],
|
||||
})),
|
||||
};
|
||||
const streamRegistry = { open: jest.fn(), bind: jest.fn(), abortEntry: jest.fn() };
|
||||
const svc = new AiChatService(
|
||||
{} as never,
|
||||
aiChatRepo as never,
|
||||
aiChatMessageRepo as never,
|
||||
{} as never, // aiChatPageSnapshotRepo (no open page -> never touched)
|
||||
aiSettings as never,
|
||||
tools as never,
|
||||
mcpClients as never,
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo (no open page)
|
||||
{} as never, // pageAccess
|
||||
{
|
||||
isAiChatDeferredToolsEnabled: () => false,
|
||||
// lockdown OFF => the degeneration detector is the anti-babble guard.
|
||||
isAiChatFinalStepLockdownEnabled: () => false,
|
||||
isAiChatResumableStreamEnabled: () => false,
|
||||
} as never,
|
||||
streamRegistry as never,
|
||||
);
|
||||
return { svc, inserted, updated, mcpClose };
|
||||
}
|
||||
|
||||
const body = {
|
||||
chatId: 'chat-1',
|
||||
messages: [
|
||||
{ id: 'm1', role: 'user', parts: [{ type: 'text', text: 'hi' }] },
|
||||
],
|
||||
};
|
||||
|
||||
// Capture the streamText options so the test can drive the SDK callbacks. The
|
||||
// returned result stub is enough for the post-streamText wiring (consumeStream +
|
||||
// pipeUIMessageStreamToResponse are no-ops here).
|
||||
function captureStreamText(): { opts: () => Record<string, any> } {
|
||||
let captured: Record<string, any> | undefined;
|
||||
streamTextMock.mockImplementation((options: Record<string, any>) => {
|
||||
captured = options;
|
||||
return {
|
||||
consumeStream: jest.fn(),
|
||||
pipeUIMessageStreamToResponse: jest.fn(),
|
||||
};
|
||||
});
|
||||
return {
|
||||
opts: () => {
|
||||
if (!captured) throw new Error('streamText was not called');
|
||||
return captured;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function drive(svc: AiChatService): Promise<void> {
|
||||
await svc.stream({
|
||||
user: { id: 'u1' } as never,
|
||||
workspace: { id: 'ws-1' } as never,
|
||||
sessionId: 's1',
|
||||
body: body as never,
|
||||
res: makeRes() as never,
|
||||
signal: new AbortController().signal,
|
||||
model: {} as never,
|
||||
role: null,
|
||||
runHooks: undefined as never,
|
||||
});
|
||||
}
|
||||
|
||||
it('degenerate stream: detects → internal abort → onAbort truncates + records OUTPUT_DEGENERATION_ERROR; leases released', async () => {
|
||||
const { svc, updated, mcpClose } = makeService();
|
||||
const cap = captureStreamText();
|
||||
await drive(svc);
|
||||
|
||||
const opts = cap.opts();
|
||||
// The turn's abort signal is the UNION of the socket/run signal and the
|
||||
// internal degeneration controller — untripped before any output.
|
||||
expect(opts.abortSignal.aborted).toBe(false);
|
||||
|
||||
// Feed a runaway "loadTools.\n" loop the way the SDK streams it: many small
|
||||
// text-delta chunks. The onChunk throttle only re-checks every ~2000 chars, so
|
||||
// deliver well past that so the detector's identical-line rule (>=25 lines)
|
||||
// and the ~2000-char throttle both fire.
|
||||
const line = 'loadTools.\n';
|
||||
let delivered = 0;
|
||||
for (let i = 0; i < 400 && !opts.abortSignal.aborted; i++) {
|
||||
opts.onChunk({ chunk: { type: 'text-delta', text: line } });
|
||||
delivered += line.length;
|
||||
}
|
||||
|
||||
// The detector must have tripped and aborted via the INTERNAL controller — the
|
||||
// reason carries the degeneration message, distinguishing it from a user Stop
|
||||
// (which aborts with no such reason) or a socket disconnect.
|
||||
expect(opts.abortSignal.aborted).toBe(true);
|
||||
expect(delivered).toBeGreaterThan(2000);
|
||||
expect(String(opts.abortSignal.reason)).toContain(
|
||||
'Output degeneration detected',
|
||||
);
|
||||
|
||||
// The SDK reacts to the aborted signal by invoking onAbort. `steps` is empty
|
||||
// (the runaway never finished a step); the in-progress runaway text is what
|
||||
// gets truncated + persisted.
|
||||
await opts.onAbort({ steps: [] });
|
||||
|
||||
// Terminal finalize = an UPDATE of the upfront-seeded assistant row (assistant
|
||||
// row was inserted upfront, so planFinalizeAssistant -> UPDATE).
|
||||
expect(updated).toHaveLength(1);
|
||||
const patch = updated[0].patch as {
|
||||
status: string;
|
||||
content: string;
|
||||
metadata: Record<string, unknown>;
|
||||
};
|
||||
// (4) status 'error' with the degeneration message — NOT 'aborted' and NOT a
|
||||
// swept 'streaming'. This distinguishes it from a user Stop / server restart.
|
||||
expect(patch.status).toBe('error');
|
||||
expect(patch.metadata.error).toBe(OUTPUT_DEGENERATION_ERROR);
|
||||
expect(patch.metadata.finishReason).toBe('error');
|
||||
// (3) the runaway tail is TRUNCATED, not the full multi-KB babble: the marker
|
||||
// is present and the persisted content is far shorter than what was streamed.
|
||||
expect(patch.content).toContain('output truncated');
|
||||
expect(patch.content.length).toBeLessThan(delivered);
|
||||
// Only a few loop reps survive (truncateDegeneratedTail keeps a handful).
|
||||
expect((patch.content.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
|
||||
|
||||
// (5) the leased external MCP client is still released on this terminal path.
|
||||
expect(mcpClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('degeneration onAbort differs from a NORMAL/user abort (no truncation, no error)', async () => {
|
||||
// Same harness, but the stream is NOT degenerate: a clean short answer, then a
|
||||
// user Stop reaches onAbort WITHOUT the degeneration controller having fired.
|
||||
const { svc, updated, mcpClose } = makeService();
|
||||
const cap = captureStreamText();
|
||||
await drive(svc);
|
||||
const opts = cap.opts();
|
||||
|
||||
opts.onChunk({ chunk: { type: 'text-delta', text: 'A normal partial answer.' } });
|
||||
// The detector never tripped -> the union signal is NOT aborted by us.
|
||||
expect(opts.abortSignal.aborted).toBe(false);
|
||||
|
||||
// A user Stop / disconnect drives onAbort with the partial (clean) text.
|
||||
await opts.onAbort({ steps: [] });
|
||||
|
||||
expect(updated).toHaveLength(1);
|
||||
const patch = updated[0].patch as {
|
||||
status: string;
|
||||
content: string;
|
||||
metadata: Record<string, unknown>;
|
||||
};
|
||||
// A normal abort persists status 'aborted' with NO error and NO truncation
|
||||
// marker — the branch is genuinely distinguished from the degeneration path.
|
||||
expect(patch.status).toBe('aborted');
|
||||
expect('error' in patch.metadata).toBe(false);
|
||||
expect(patch.content).toBe('A normal partial answer.');
|
||||
expect(patch.content).not.toContain('output truncated');
|
||||
// Cleanup still runs on the normal abort path too.
|
||||
expect(mcpClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
/**
|
||||
* Empty-turn marker (#444): onFinish appends STEP_LIMIT_NO_ANSWER_MARKER only
|
||||
* when the turn burned ALL its steps (steps.length >= MAX_AGENT_STEPS) AND never
|
||||
* produced any text. The negative: a normal turn ending WITH text is left alone.
|
||||
*/
|
||||
it('empty turn (no text + steps exhausted) persists the STEP_LIMIT_NO_ANSWER_MARKER', async () => {
|
||||
const { svc, updated } = makeService();
|
||||
const cap = captureStreamText();
|
||||
await drive(svc);
|
||||
const opts = cap.opts();
|
||||
|
||||
// MAX_AGENT_STEPS text-less steps (only tool calls) => step-exhausted, no text.
|
||||
const steps = Array.from({ length: MAX_AGENT_STEPS }, () => ({
|
||||
text: '',
|
||||
toolCalls: [{ toolCallId: 'c1', toolName: 'searchPages', input: {} }],
|
||||
toolResults: [
|
||||
{ toolCallId: 'c1', toolName: 'searchPages', output: { hits: [] } },
|
||||
],
|
||||
}));
|
||||
await opts.onFinish({
|
||||
text: '',
|
||||
finishReason: 'tool-calls',
|
||||
totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
|
||||
usage: { inputTokens: 1, outputTokens: 1 },
|
||||
steps,
|
||||
});
|
||||
|
||||
expect(updated).toHaveLength(1);
|
||||
const patch = updated[0].patch as { status: string; content: string };
|
||||
expect(patch.status).toBe('completed');
|
||||
// The synthetic marker is the trailing text of the persisted content.
|
||||
expect(patch.content).toContain(STEP_LIMIT_NO_ANSWER_MARKER);
|
||||
});
|
||||
|
||||
it('normal turn ending WITH text does NOT get the empty-turn marker', async () => {
|
||||
const { svc, updated } = makeService();
|
||||
const cap = captureStreamText();
|
||||
await drive(svc);
|
||||
const opts = cap.opts();
|
||||
|
||||
// A single step that produced a real answer, well under the step cap.
|
||||
const steps = [
|
||||
{ text: 'Here is the finished answer.', toolCalls: [], toolResults: [] },
|
||||
];
|
||||
await opts.onFinish({
|
||||
text: 'Here is the finished answer.',
|
||||
finishReason: 'stop',
|
||||
totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
|
||||
usage: { inputTokens: 1, outputTokens: 1 },
|
||||
steps,
|
||||
});
|
||||
|
||||
expect(updated).toHaveLength(1);
|
||||
const patch = updated[0].patch as { status: string; content: string };
|
||||
expect(patch.status).toBe('completed');
|
||||
expect(patch.content).toBe('Here is the finished answer.');
|
||||
expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
|
||||
});
|
||||
|
||||
it('step-exhausted turn that DID produce text keeps the text, no marker (guards the AND)', async () => {
|
||||
// Exhausting the step budget alone must NOT append the marker when SOME step
|
||||
// produced text — the marker keys off "no text" too. Drive the real onFinish
|
||||
// with MAX_AGENT_STEPS steps where the last one carries the answer.
|
||||
const { svc, updated } = makeService();
|
||||
const cap = captureStreamText();
|
||||
await drive(svc);
|
||||
const opts = cap.opts();
|
||||
|
||||
const steps = Array.from({ length: MAX_AGENT_STEPS }, (_, i) => ({
|
||||
text: i === MAX_AGENT_STEPS - 1 ? 'Final synthesized answer.' : '',
|
||||
toolCalls:
|
||||
i === MAX_AGENT_STEPS - 1
|
||||
? []
|
||||
: [{ toolCallId: `c${i}`, toolName: 'searchPages', input: {} }],
|
||||
toolResults:
|
||||
i === MAX_AGENT_STEPS - 1
|
||||
? []
|
||||
: [{ toolCallId: `c${i}`, toolName: 'searchPages', output: {} }],
|
||||
}));
|
||||
await opts.onFinish({
|
||||
text: 'Final synthesized answer.',
|
||||
finishReason: 'stop',
|
||||
totalUsage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 },
|
||||
usage: { inputTokens: 1, outputTokens: 1 },
|
||||
steps,
|
||||
});
|
||||
|
||||
expect(updated).toHaveLength(1);
|
||||
const patch = updated[0].patch as { content: string };
|
||||
expect(patch.content).toContain('Final synthesized answer.');
|
||||
expect(patch.content).not.toContain(STEP_LIMIT_NO_ANSWER_MARKER);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,24 +52,11 @@ import {
|
||||
startSseHeartbeat,
|
||||
stripStreamingHopByHopHeaders,
|
||||
} from './sse-resilience';
|
||||
import {
|
||||
isDegenerateOutput,
|
||||
truncateDegeneratedTail,
|
||||
} from './output-degeneration';
|
||||
|
||||
// Max agent steps per turn. One step = one model generation; a step that calls
|
||||
// tools is followed by another step carrying the tool results. Raised from 8 so
|
||||
// multi-search research questions are not cut off mid-investigation, then from 20
|
||||
// to 50 (#444) so read-heavy turns (e.g. dozens of searchInPage sweeps) do not
|
||||
// exhaust the budget before acting.
|
||||
const MAX_AGENT_STEPS = 50;
|
||||
|
||||
// How many steps before the LAST one the step-budget warning starts firing
|
||||
// (#444). At MAX-STEP_BUDGET_WARNING_LEAD .. MAX-2 the model is told to stop
|
||||
// exploring and start acting, with the remaining count decreasing each step; the
|
||||
// last step (MAX-1) has its own final nudge / lockdown instead (see
|
||||
// prepareAgentStep).
|
||||
const STEP_BUDGET_WARNING_LEAD = 6;
|
||||
// multi-search research questions are not cut off mid-investigation.
|
||||
const MAX_AGENT_STEPS = 20;
|
||||
|
||||
// Wall-clock ceiling for building the external MCP toolset during the per-turn
|
||||
// setup phase (before streamText owns the lifecycle). Defense-in-depth ABOVE the
|
||||
@@ -95,69 +82,16 @@ const FINAL_STEP_INSTRUCTION =
|
||||
'language. If the information is incomplete, say so explicitly: summarize ' +
|
||||
'what you found, what is still missing, and give your best partial conclusion.';
|
||||
|
||||
// SOFT final-step nudge (#444), used when the final-step lockdown toggle is OFF
|
||||
// (the new default). Unlike FINAL_STEP_INSTRUCTION it does NOT strip tools
|
||||
// (toolChoice stays untouched), so the model is never forced into a tool-less
|
||||
// state mid-work — that tool-stripping is what triggered the 255KB token-loop
|
||||
// degeneration incident. It only asks the model to finish with a text summary.
|
||||
const FINAL_STEP_NUDGE =
|
||||
'This is the LAST step of this turn. Write your final answer to the user now.\n' +
|
||||
'You may still call tools, but the turn ends after this step either way —\n' +
|
||||
'prefer finishing with a clear text summary of what was done and what remains.';
|
||||
|
||||
// Synthetic marker text appended in onFinish when a step-exhausted turn produced
|
||||
// NO text at all (#444, mitigates the "empty turn" the lockdown used to prevent
|
||||
// when the toggle is OFF). Makes the exhausted-without-answer state explicit to
|
||||
// the user and, on replay, to the model on the next turn.
|
||||
const STEP_LIMIT_NO_ANSWER_MARKER =
|
||||
'(Достигнут лимит шагов — итоговый ответ не сформулирован; работа могла ' +
|
||||
'остаться незавершённой. Напишите «продолжай», чтобы агент продолжил.)';
|
||||
|
||||
// Reason recorded in ai_chat_runs.error / the assistant row when the token-
|
||||
// degeneration detector (#444) aborts a run. Distinct from a user Stop (no error)
|
||||
// and from a server restart ('streaming' -> swept to 'aborted' with no message).
|
||||
const OUTPUT_DEGENERATION_ERROR =
|
||||
'Output degeneration detected (repeated token loop)';
|
||||
|
||||
/**
|
||||
* Compute the step-budget warning text (#444), or '' when this step is outside
|
||||
* the warning band. The warning fires on steps
|
||||
* MAX_AGENT_STEPS-STEP_BUDGET_WARNING_LEAD .. MAX_AGENT_STEPS-2 (NOT the last
|
||||
* step, which has its own final nudge/lockdown), telling the model to stop
|
||||
* exploring and start acting. `N` is the number of tool-use steps still
|
||||
* remaining (`MAX_AGENT_STEPS - 1 - stepNumber`), so it decreases toward the
|
||||
* end. Pure.
|
||||
*/
|
||||
export function stepBudgetWarning(stepNumber: number): string {
|
||||
const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
|
||||
const inBand = stepNumber >= MAX_AGENT_STEPS - STEP_BUDGET_WARNING_LEAD;
|
||||
if (isLastStep || !inBand) return '';
|
||||
const remaining = MAX_AGENT_STEPS - 1 - stepNumber;
|
||||
return (
|
||||
`Only ${remaining} tool-use steps remain in this turn. Stop exploring and start acting now\n` +
|
||||
'(make the edits / create the comments / produce results). Leave room to finish\n' +
|
||||
'with a final text answer.'
|
||||
);
|
||||
}
|
||||
|
||||
// Pure, unit-testable: decide per-step overrides. Responsibilities:
|
||||
// 1. Final-step handling. Two modes, chosen by `finalStepLockdownEnabled`:
|
||||
// - toggle ON (legacy): on the final allowed step force a text-only
|
||||
// synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS
|
||||
// — it takes precedence over the deferred-tool narrowing below.
|
||||
// - toggle OFF (new default, #444): do NOT touch toolChoice — tools stay
|
||||
// available on every step incl. the last, so the model is never stripped
|
||||
// of its tools mid-work (the cause of the token-loop degeneration
|
||||
// incident). A SOFT nudge (FINAL_STEP_NUDGE) is appended to `system`, and
|
||||
// the deferred-tool `activeTools` narrowing still applies to the last step
|
||||
// (both `activeTools` and `system` are returned together).
|
||||
// 2. Step-budget warning (#444): on steps in the warning band (but not the
|
||||
// last, which has its own nudge/lockdown) append stepBudgetWarning(...) to
|
||||
// `system` so the model starts acting before it runs out of steps.
|
||||
// 3. Deferred tool visibility (#332): when `deferredEnabled`, expose only the
|
||||
// CORE tools + loadTools + whatever loadTools has activated so far this turn
|
||||
// (`activatedTools`), via `activeTools`. Deferred tools stay in the
|
||||
// <tool_catalog> until the model loads them.
|
||||
// Pure, unit-testable: decide per-step overrides. Two responsibilities:
|
||||
// 1. Final-step lockdown (always): on the final allowed step force a text-only
|
||||
// synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
|
||||
// it takes precedence over the deferred-tool narrowing below.
|
||||
// 2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
|
||||
// step, expose only the CORE tools + loadTools + whatever loadTools has
|
||||
// activated so far this turn (`activatedTools`), via `activeTools`. Deferred
|
||||
// tools stay in the <tool_catalog> until the model loads them.
|
||||
// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
|
||||
// steps (all tools active), lockdown on the final step.
|
||||
//
|
||||
// `system` is the in-scope system prompt; we CONCATENATE so the original
|
||||
// persona/context is preserved — a bare `system` override would REPLACE the
|
||||
@@ -173,53 +107,31 @@ export function prepareAgentStep(
|
||||
system: string,
|
||||
activatedTools: ReadonlySet<string> | readonly string[] = [],
|
||||
deferredEnabled = false,
|
||||
finalStepLockdownEnabled = false,
|
||||
):
|
||||
| { toolChoice: 'none'; system: string }
|
||||
| { activeTools: string[]; system?: string }
|
||||
| { system: string }
|
||||
| { activeTools: string[] }
|
||||
| undefined {
|
||||
const isLastStep = stepNumber >= MAX_AGENT_STEPS - 1;
|
||||
|
||||
// Legacy final-step lockdown (toggle ON): text-only synthesis. WINS over the
|
||||
// deferred narrowing AND drops tools for this step.
|
||||
if (isLastStep && finalStepLockdownEnabled) {
|
||||
// Final-step lockdown WINS (applies regardless of the deferred toggle).
|
||||
if (stepNumber >= MAX_AGENT_STEPS - 1) {
|
||||
return {
|
||||
toolChoice: 'none',
|
||||
system: `${system}\n\n${FINAL_STEP_INSTRUCTION}`,
|
||||
};
|
||||
}
|
||||
|
||||
// Compute the extra system text for this step: the soft final nudge on the last
|
||||
// step (toggle OFF), or the step-budget warning in the warning band. At most one
|
||||
// of these applies (stepBudgetWarning returns '' on the last step).
|
||||
const extra = isLastStep ? FINAL_STEP_NUDGE : stepBudgetWarning(stepNumber);
|
||||
const systemForStep = extra ? `${system}\n\n${extra}` : undefined;
|
||||
|
||||
// Deferred tool loading: narrow this step's visible tools to CORE + loadTools +
|
||||
// the tools already activated this turn. Applies on EVERY step incl. the last
|
||||
// (toggle OFF), so the model keeps its core tools available while being nudged
|
||||
// to finish. Return `system` alongside `activeTools` when we have extra text.
|
||||
// Deferred tool loading: narrow this step's visible tools to CORE + loadTools
|
||||
// + the tools already activated this turn.
|
||||
if (deferredEnabled) {
|
||||
const activated = Array.isArray(activatedTools)
|
||||
? activatedTools
|
||||
: [...activatedTools];
|
||||
const activeTools = [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated];
|
||||
return systemForStep ? { activeTools, system: systemForStep } : { activeTools };
|
||||
return {
|
||||
activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
|
||||
};
|
||||
}
|
||||
|
||||
// Deferred OFF: all tools stay active; only append the extra system text (if any).
|
||||
return systemForStep ? { system: systemForStep } : undefined;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export {
|
||||
MAX_AGENT_STEPS,
|
||||
STEP_BUDGET_WARNING_LEAD,
|
||||
FINAL_STEP_INSTRUCTION,
|
||||
FINAL_STEP_NUDGE,
|
||||
STEP_LIMIT_NO_ANSWER_MARKER,
|
||||
OUTPUT_DEGENERATION_ERROR,
|
||||
};
|
||||
export { MAX_AGENT_STEPS, FINAL_STEP_INSTRUCTION };
|
||||
|
||||
// Pure, unit-testable post-processing for a model-generated title (#199): trim
|
||||
// whitespace, strip a single pair of surrounding quotes the model often adds,
|
||||
@@ -978,12 +890,6 @@ export class AiChatService implements OnModuleInit {
|
||||
// tools (fat/rare in-app tools + ALL external MCP tools) load on demand. When
|
||||
// OFF, every tool is active and nothing below changes.
|
||||
const deferredEnabled = this.environment.isAiChatDeferredToolsEnabled();
|
||||
// Final-step lockdown toggle (#444). Default OFF: the last step keeps its
|
||||
// tools and gets only a soft nudge (prepareAgentStep), and the token-
|
||||
// degeneration detector (onChunk below) is the anti-babble guard. ON =
|
||||
// legacy tool-stripping lockdown on the last step.
|
||||
const finalStepLockdownEnabled =
|
||||
this.environment.isAiChatFinalStepLockdownEnabled();
|
||||
|
||||
let system: string;
|
||||
let docmostTools: Awaited<ReturnType<AiChatToolsService['forUser']>>;
|
||||
@@ -1072,16 +978,6 @@ export class AiChatService implements OnModuleInit {
|
||||
const capturedSteps: StepLike[] = [];
|
||||
let inProgressText = '';
|
||||
|
||||
// Token-degeneration guard (#444). When the final-step lockdown is OFF, a
|
||||
// runaway repetition loop (the 255KB "loadTools." incident) is aborted via
|
||||
// this internal controller, unioned with the run/socket signal below. The
|
||||
// detector runs on `inProgressText` in onChunk, throttled by growth so the
|
||||
// pure rules only fire every ~DEGENERATION_CHECK_STEP bytes.
|
||||
const degenerationController = new AbortController();
|
||||
let degenerationDetected = false;
|
||||
let lastDegenerationCheckLen = 0;
|
||||
const DEGENERATION_CHECK_STEP = 2000;
|
||||
|
||||
// Step-granular durability (#183): create the assistant row UPFRONT in the
|
||||
// 'streaming' state (before any token), then UPDATE it as each step finishes
|
||||
// and finalize it once on the terminal callback. If the process dies
|
||||
@@ -1222,21 +1118,11 @@ export class AiChatService implements OnModuleInit {
|
||||
// further tool calls and appends a synthesis instruction on that step,
|
||||
// concatenated onto the original `system` so the persona is preserved.
|
||||
prepareStep: ({ stepNumber }) =>
|
||||
prepareAgentStep(
|
||||
stepNumber,
|
||||
system,
|
||||
activatedTools,
|
||||
deferredEnabled,
|
||||
finalStepLockdownEnabled,
|
||||
),
|
||||
prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
|
||||
// #184: the RUN's signal (explicit-stop) when a run wraps this turn, else
|
||||
// the socket-bound signal (legacy). A browser disconnect aborts only in
|
||||
// the legacy path. #444: UNION it with the internal degeneration signal
|
||||
// so a detected token-loop aborts the run too (AbortSignal.any — Node 20.3+).
|
||||
abortSignal: AbortSignal.any([
|
||||
effectiveSignal,
|
||||
degenerationController.signal,
|
||||
]),
|
||||
// the legacy path.
|
||||
abortSignal: effectiveSignal,
|
||||
onChunk: ({ chunk }) => {
|
||||
// DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
|
||||
// output chunk means the stream is actively emitting bytes; track first
|
||||
@@ -1246,29 +1132,7 @@ export class AiChatService implements OnModuleInit {
|
||||
lastModelChunkAt = now;
|
||||
// 'text-delta' is the assistant's prose; tool-call args are separate chunk
|
||||
// types — so this mirrors exactly what streams to the client.
|
||||
if (chunk.type === 'text-delta') {
|
||||
inProgressText += chunk.text;
|
||||
// Token-degeneration guard (#444). Throttled: only re-run the pure
|
||||
// rules once the text has grown ~DEGENERATION_CHECK_STEP bytes since
|
||||
// the last check, so the tail heuristics cost is amortized. On a
|
||||
// trigger, abort the run ONCE with a distinguishable reason.
|
||||
if (
|
||||
!degenerationDetected &&
|
||||
inProgressText.length - lastDegenerationCheckLen >=
|
||||
DEGENERATION_CHECK_STEP
|
||||
) {
|
||||
lastDegenerationCheckLen = inProgressText.length;
|
||||
if (isDegenerateOutput(inProgressText)) {
|
||||
degenerationDetected = true;
|
||||
this.logger.warn(
|
||||
`AI chat stream aborted (chat ${chatId}): ${OUTPUT_DEGENERATION_ERROR}`,
|
||||
);
|
||||
degenerationController.abort(
|
||||
new Error(OUTPUT_DEGENERATION_ERROR),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (chunk.type === 'text-delta') inProgressText += chunk.text;
|
||||
},
|
||||
onStepFinish: (step) => {
|
||||
// The finished step's full text is now in `step.text`; fold it in and reset
|
||||
@@ -1310,22 +1174,8 @@ export class AiChatService implements OnModuleInit {
|
||||
// plain-text projection (full-text search / fallback). A multi-step
|
||||
// turn's `content` therefore now holds all steps' prose, not just the
|
||||
// last block.
|
||||
// Empty-turn mitigation (#444, toggle OFF). If the turn burned all its
|
||||
// steps WITHOUT ever producing text (every step's text is empty) and
|
||||
// the model stopped because it hit the step cap, there is no answer to
|
||||
// show — the lockdown used to force one. Append a synthetic marker as
|
||||
// the trailing text so the exhausted-without-answer state is explicit
|
||||
// to the user and, on replay, to the model next turn. `flushAssistant`
|
||||
// takes this as the `inProgressText` trailing text arg (empty here
|
||||
// otherwise). `stepCountIs(MAX_AGENT_STEPS)` surfaces as
|
||||
// finishReason === 'tool-calls' (or a length/other cap), so we key off
|
||||
// "no text produced" rather than a single finishReason string.
|
||||
const producedText = (steps as StepLike[]).some((s) => s.text?.trim());
|
||||
const stepExhausted = steps.length >= MAX_AGENT_STEPS;
|
||||
const emptyTurnMarker =
|
||||
!producedText && stepExhausted ? STEP_LIMIT_NO_ANSWER_MARKER : '';
|
||||
await finalizeAssistant(
|
||||
flushAssistant(steps as StepLike[], emptyTurnMarker, 'completed', {
|
||||
flushAssistant(steps as StepLike[], '', 'completed', {
|
||||
finishReason: finishReason as string,
|
||||
usage: totalUsage as StreamUsage,
|
||||
contextTokens:
|
||||
@@ -1402,30 +1252,6 @@ export class AiChatService implements OnModuleInit {
|
||||
await snapshotTurnEnd();
|
||||
},
|
||||
onAbort: async ({ steps }) => {
|
||||
// #444: distinguish a degeneration abort (our internal controller) from
|
||||
// a user Stop / disconnect. On degeneration we truncate the runaway tail
|
||||
// before persist (so hundreds of KB of garbage never reach the DB /
|
||||
// replay) and record it as an ERROR with a clear, distinguishable reason
|
||||
// — NOT a bare 'aborted' (a user Stop) and NOT a swept 'streaming' (a
|
||||
// server restart).
|
||||
if (degenerationDetected) {
|
||||
const truncated = truncateDegeneratedTail(inProgressText);
|
||||
await finalizeAssistant(
|
||||
flushAssistant(capturedSteps, truncated, 'error', {
|
||||
error: OUTPUT_DEGENERATION_ERROR,
|
||||
pageChanged,
|
||||
}),
|
||||
);
|
||||
if (runId)
|
||||
await runHooks?.onSettled?.(
|
||||
runId,
|
||||
'error',
|
||||
OUTPUT_DEGENERATION_ERROR,
|
||||
);
|
||||
await closeExternalClients();
|
||||
await snapshotTurnEnd();
|
||||
return;
|
||||
}
|
||||
const partialChars =
|
||||
capturedSteps.reduce((n, s) => n + (s.text?.length ?? 0), 0) +
|
||||
inProgressText.length;
|
||||
|
||||
@@ -1,182 +0,0 @@
|
||||
import {
|
||||
hasRepeatedLineRun,
|
||||
hasPeriodicTail,
|
||||
isDegenerateOutput,
|
||||
truncateDegeneratedTail,
|
||||
REPEATED_LINES_THRESHOLD,
|
||||
MIN_PERIOD_REPEATS,
|
||||
} from './output-degeneration';
|
||||
|
||||
/**
|
||||
* Unit tests for the token-degeneration detector (#444) — the sole anti-babble
|
||||
* guard once the final-step lockdown is OFF. The two rules must fire on real
|
||||
* degeneration (the "loadTools." incident, a no-newline repeat) and MUST NOT fire
|
||||
* on legitimate long output (edit lists, tables, code).
|
||||
*/
|
||||
describe('hasRepeatedLineRun (rule 1: identical-line run)', () => {
|
||||
it('POSITIVE: fires on "loadTools.\\n" repeated many times (the incident)', () => {
|
||||
const text = 'Here is my plan.\n' + 'loadTools.\n'.repeat(300);
|
||||
expect(hasRepeatedLineRun(text)).toBe(true);
|
||||
expect(isDegenerateOutput(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('POSITIVE: fires at exactly the threshold', () => {
|
||||
const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD);
|
||||
expect(hasRepeatedLineRun(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('NEGATIVE: does NOT fire just below the threshold', () => {
|
||||
// threshold-1 identical lines followed by a distinct line.
|
||||
const text = 'x\n'.repeat(REPEATED_LINES_THRESHOLD - 1) + 'done\n';
|
||||
expect(hasRepeatedLineRun(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a long edit list of DISTINCT lines never trips', () => {
|
||||
const lines: string[] = [];
|
||||
for (let i = 0; i < 200; i++) lines.push(`- edited section ${i}: fixed typo`);
|
||||
const text = lines.join('\n');
|
||||
expect(hasRepeatedLineRun(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a markdown table with blank separators does not trip', () => {
|
||||
// Repeated identical rows are unusual, but blank lines break any run.
|
||||
const block = ['| a | b |', '| - | - |', '', '| a | b |', ''];
|
||||
const text = Array.from({ length: 60 }, () => block.join('\n')).join('\n');
|
||||
expect(hasRepeatedLineRun(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: blank lines do NOT count toward a run', () => {
|
||||
const text = '\n'.repeat(100);
|
||||
expect(hasRepeatedLineRun(text)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasPeriodicTail (rule 2: no-newline suffix periodicity)', () => {
|
||||
it('POSITIVE: fires on a single char repeated with no newlines', () => {
|
||||
const text = 'answer: ' + 'a'.repeat(500);
|
||||
expect(hasPeriodicTail(text)).toBe(true);
|
||||
expect(isDegenerateOutput(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('POSITIVE: fires on a multi-char block repeat with no newlines', () => {
|
||||
const text = 'prefix ' + 'abcdef'.repeat(100);
|
||||
expect(hasPeriodicTail(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('POSITIVE: at least MIN_PERIOD_REPEATS repeats of a small block', () => {
|
||||
const text = 'go'.repeat(MIN_PERIOD_REPEATS);
|
||||
expect(hasPeriodicTail(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('NEGATIVE: prose does not look periodic', () => {
|
||||
const text =
|
||||
'The quick brown fox jumps over the lazy dog while the sun sets slowly ' +
|
||||
'behind the distant mountains and the river winds through the valley below.';
|
||||
expect(hasPeriodicTail(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a long code block is not flagged', () => {
|
||||
const code = `
|
||||
function compute(values) {
|
||||
let total = 0;
|
||||
for (const v of values) {
|
||||
total += v * 2;
|
||||
}
|
||||
return total / values.length;
|
||||
}
|
||||
export const helper = (x) => x + 1;
|
||||
const config = { retries: 3, timeout: 5000, backoff: 'exp' };
|
||||
`.repeat(3);
|
||||
expect(isDegenerateOutput(code)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a short string well under the repeat count is safe', () => {
|
||||
expect(hasPeriodicTail('ababab')).toBe(false);
|
||||
});
|
||||
|
||||
// Regression (#444): a trivial single-char period (p===1) must NOT flag
|
||||
// legitimate divider/underline/whitespace runs. These are common in real
|
||||
// model output and previously false-positived at ~20 identical chars, aborting
|
||||
// the run and truncating output. They must all be treated as clean.
|
||||
it('NEGATIVE: a markdown horizontal rule is not flagged', () => {
|
||||
const text = 'text\n' + '-'.repeat(40);
|
||||
expect(hasPeriodicTail(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a setext heading underline is not flagged', () => {
|
||||
const text = 'Title\n' + '='.repeat(30);
|
||||
expect(hasPeriodicTail(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: a box-drawing divider with no trailing newline is not flagged', () => {
|
||||
const text = 'done ' + '─'.repeat(50);
|
||||
expect(hasPeriodicTail(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
it('NEGATIVE: trailing spaces are not flagged', () => {
|
||||
const text = 'answer' + ' '.repeat(40);
|
||||
expect(hasPeriodicTail(text)).toBe(false);
|
||||
expect(isDegenerateOutput(text)).toBe(false);
|
||||
});
|
||||
|
||||
// TRIVIAL_MIN_REPEATS boundary (#444 review). The monochar-tail branch fires at
|
||||
// EXACTLY 60 identical trailing chars (`run >= TRIVIAL_MIN_REPEATS`), so 59 is
|
||||
// clean and 60 trips. These pin the `>=` and MUST fail if the comparison is
|
||||
// flipped to `>` (the surviving mutation). The value 60 is HARD-CODED here on
|
||||
// purpose: TRIVIAL_MIN_REPEATS is a private constant and the assert must lock
|
||||
// the literal boundary the reviewer named, not track a constant edit.
|
||||
it('NEGATIVE: 59 identical trailing chars is one below the monochar threshold', () => {
|
||||
expect(hasPeriodicTail('x'.repeat(59))).toBe(false);
|
||||
expect(isDegenerateOutput('x'.repeat(59))).toBe(false);
|
||||
});
|
||||
|
||||
it('POSITIVE: 60 identical trailing chars hits the monochar threshold exactly', () => {
|
||||
// Fails if `run >= TRIVIAL_MIN_REPEATS` is mutated to `run > …`.
|
||||
expect(hasPeriodicTail('x'.repeat(60))).toBe(true);
|
||||
expect(isDegenerateOutput('x'.repeat(60))).toBe(true);
|
||||
});
|
||||
|
||||
// Positive counterparts: a GENUINE single-char runaway (hundreds+ of repeats)
|
||||
// and the real incident (period>=2, "loadTools." ×N) must still fire.
|
||||
it('POSITIVE: a genuine single-char runaway is still flagged', () => {
|
||||
const text = 'x'.repeat(5000);
|
||||
expect(hasPeriodicTail(text)).toBe(true);
|
||||
expect(isDegenerateOutput(text)).toBe(true);
|
||||
});
|
||||
|
||||
it('POSITIVE: the "loadTools." incident (period>=2) is still flagged', () => {
|
||||
const text = 'loadTools.'.repeat(500);
|
||||
expect(hasPeriodicTail(text)).toBe(true);
|
||||
expect(isDegenerateOutput(text)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('truncateDegeneratedTail', () => {
|
||||
it('collapses a repeated-line loop to a few reps + marker', () => {
|
||||
const text = 'plan\n' + 'loadTools.\n'.repeat(20000);
|
||||
const out = truncateDegeneratedTail(text);
|
||||
expect(out.length).toBeLessThan(text.length);
|
||||
expect(out).toContain('output truncated');
|
||||
// Keeps the leading context and a few loop reps.
|
||||
expect(out).toContain('plan');
|
||||
expect((out.match(/loadTools\./g) ?? []).length).toBeLessThan(10);
|
||||
});
|
||||
|
||||
it('collapses a no-newline periodic loop to a few blocks + marker', () => {
|
||||
const text = 'answer: ' + 'xy'.repeat(50000);
|
||||
const out = truncateDegeneratedTail(text);
|
||||
expect(out.length).toBeLessThan(text.length);
|
||||
expect(out).toContain('output truncated');
|
||||
expect(out).toContain('answer:');
|
||||
});
|
||||
|
||||
it('returns non-degenerate text unchanged (by identity)', () => {
|
||||
const text = 'A perfectly normal, finished assistant answer.';
|
||||
expect(truncateDegeneratedTail(text)).toBe(text);
|
||||
});
|
||||
});
|
||||
@@ -1,189 +0,0 @@
|
||||
/**
|
||||
* Token-degeneration detector for the in-app agent stream (#444).
|
||||
*
|
||||
* When the final-step lockdown is OFF (the new default) there is no toolChoice
|
||||
* override to strip the model's tools mid-work, so the anti-babble safety net is
|
||||
* this detector. It watches the accumulating assistant text and, on a runaway
|
||||
* repetition loop (the 255KB "loadTools." incident), aborts the run.
|
||||
*
|
||||
* Both rules are PURE functions of the text tail so they are cheap to run every
|
||||
* few KB and are unit-testable in isolation. They operate on the TAIL only
|
||||
* (`TAIL_WINDOW` chars) so the cost is bounded regardless of how long the turn is.
|
||||
*/
|
||||
|
||||
/** How many trailing chars of the accumulated text the rules inspect. */
|
||||
export const TAIL_WINDOW = 3000;
|
||||
|
||||
/** Rule 1 threshold: minimum consecutive identical non-empty lines to trigger. */
|
||||
export const REPEATED_LINES_THRESHOLD = 25;
|
||||
|
||||
/** Rule 2: maximum length of a repeating block considered for periodicity. */
|
||||
export const MAX_PERIOD_LEN = 150;
|
||||
|
||||
/** Rule 2: minimum number of consecutive block repeats to trigger. */
|
||||
export const MIN_PERIOD_REPEATS = 20;
|
||||
|
||||
/**
|
||||
* Rule 1 — ≥`REPEATED_LINES_THRESHOLD` consecutive IDENTICAL non-empty lines at
|
||||
* the tail. Catches the classic newline-delimited loop ("loadTools.\n" ×N).
|
||||
* Blank lines break a run (a table / list with blank separators never trips it);
|
||||
* a run of ordinary distinct lines (an edit list, code) never reaches the count.
|
||||
*
|
||||
* NB: `REPEATED_LINES_THRESHOLD` (25) is only THIS rule's own trigger, not the
|
||||
* effective floor for detecting a repeated-line loop. In practice a newline-
|
||||
* delimited repeat also has a fixed period (line + '\n'), so rule 2 catches it
|
||||
* via periodicity at `MIN_PERIOD_REPEATS` (20) repeats — the two rules combine
|
||||
* (see `isDegenerateOutput`), so the effective lower bound for a short identical
|
||||
* line loop is ~20, not 25. Pure.
|
||||
*/
|
||||
export function hasRepeatedLineRun(
|
||||
text: string,
|
||||
threshold = REPEATED_LINES_THRESHOLD,
|
||||
): boolean {
|
||||
const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
|
||||
const lines = tail.split('\n');
|
||||
let run = 1;
|
||||
let prev: string | null = null;
|
||||
for (const line of lines) {
|
||||
if (line.length > 0 && line === prev) {
|
||||
run += 1;
|
||||
if (run >= threshold) return true;
|
||||
} else {
|
||||
run = 1;
|
||||
}
|
||||
prev = line;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rule 2 — cheap suffix-periodicity check: the tail ends in
|
||||
* ≥`MIN_PERIOD_REPEATS` back-to-back repeats of a single block of length
|
||||
* ≤`MAX_PERIOD_LEN`. Catches a no-newline repeat ("abcabcabc…") the line rule
|
||||
* misses. For each candidate period length p we verify the last `repeats*p`
|
||||
* chars are p-periodic; we stop at the smallest p that satisfies the repeat
|
||||
* count. Bounded by MAX_PERIOD_LEN × TAIL_WINDOW comparisons — negligible. Pure.
|
||||
*/
|
||||
export function hasPeriodicTail(
|
||||
text: string,
|
||||
maxPeriod = MAX_PERIOD_LEN,
|
||||
minRepeats = MIN_PERIOD_REPEATS,
|
||||
): boolean {
|
||||
const tail = text.length > TAIL_WINDOW ? text.slice(-TAIL_WINDOW) : text;
|
||||
const n = tail.length;
|
||||
// Not even the shortest possible loop fits in the tail.
|
||||
if (n < minRepeats) return false;
|
||||
// A tail of ONE repeated char (a "trivial period") is common in LEGIT output —
|
||||
// markdown rules (----/====), setext underlines, box-drawing dividers,
|
||||
// trailing spaces routinely produce 20–50 identical chars. Such a run is
|
||||
// p-periodic for EVERY p, so it would otherwise trip the block rule at p>=2
|
||||
// too, not just p===1. We therefore split the check: a monochar tail needs far
|
||||
// more repeats (a real single-char babble loop produces hundreds-to-thousands;
|
||||
// 60 is well above any realistic divider yet a fifth of TAIL_WINDOW), while a
|
||||
// genuine multi-char block repeat (>=2 distinct chars, e.g. the "loadTools."
|
||||
// incident, period ~10) keeps the normal MIN_PERIOD_REPEATS threshold.
|
||||
const TRIVIAL_MIN_REPEATS = 60;
|
||||
|
||||
// Monochar-tail check (the trivial-period case): count the trailing run of one
|
||||
// identical char and require TRIVIAL_MIN_REPEATS of them.
|
||||
{
|
||||
const last = tail[n - 1];
|
||||
let run = 1;
|
||||
for (let i = n - 2; i >= 0 && tail[i] === last; i--) run++;
|
||||
if (run >= TRIVIAL_MIN_REPEATS) return true;
|
||||
}
|
||||
|
||||
const maxP = maxPeriod;
|
||||
for (let p = 2; p <= maxP; p++) {
|
||||
// Verify the last (minRepeats*p) chars are p-periodic AND not monochar (a
|
||||
// monochar span is the trivial case handled above, so skip it here to avoid
|
||||
// re-flagging a legit divider at a composite period).
|
||||
const span = minRepeats * p;
|
||||
// Not enough tail to hold this many repeats of this period.
|
||||
if (span > n) continue;
|
||||
const start = n - span;
|
||||
let periodic = true;
|
||||
let multiChar = false;
|
||||
for (let i = n - 1; i >= start + p; i--) {
|
||||
if (tail[i] !== tail[i - p]) {
|
||||
periodic = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!periodic) continue;
|
||||
// Confirm the block itself has >=2 distinct chars (else it's monochar).
|
||||
for (let i = start + 1; i < n; i++) {
|
||||
if (tail[i] !== tail[start]) {
|
||||
multiChar = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (multiChar) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Combined guard used by the stream's onChunk: true when EITHER rule fires.
|
||||
* Pure — the caller owns the abort side effect.
|
||||
*/
|
||||
export function isDegenerateOutput(text: string): boolean {
|
||||
return hasRepeatedLineRun(text) || hasPeriodicTail(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* Truncate a degenerated tail before persist so hundreds of KB of garbage never
|
||||
* reach the DB / replay (#444). Keeps everything up to and including the FIRST
|
||||
* `keepRepeats` repeats of the detected loop, then appends a short marker. If no
|
||||
* loop is detected the text is returned unchanged (by identity).
|
||||
*
|
||||
* Implementation: find the shortest tail period (same check as hasPeriodicTail),
|
||||
* keep the prefix before the loop plus `keepRepeats` copies of the block, drop
|
||||
* the rest. This is best-effort cosmetic trimming; correctness does not depend on
|
||||
* finding the exact minimal loop. Pure.
|
||||
*/
|
||||
export function truncateDegeneratedTail(
|
||||
text: string,
|
||||
keepRepeats = 3,
|
||||
): string {
|
||||
const marker = '\n…[output truncated: repeated token loop detected]';
|
||||
// Try the line rule first: collapse a long run of identical lines.
|
||||
const lines = text.split('\n');
|
||||
let runStart = -1;
|
||||
let run = 1;
|
||||
for (let i = 1; i < lines.length; i++) {
|
||||
if (lines[i].length > 0 && lines[i] === lines[i - 1]) {
|
||||
if (run === 1) runStart = i - 1;
|
||||
run += 1;
|
||||
if (run >= REPEATED_LINES_THRESHOLD) {
|
||||
const kept = lines.slice(0, runStart + keepRepeats).join('\n');
|
||||
return kept + marker;
|
||||
}
|
||||
} else {
|
||||
run = 1;
|
||||
runStart = -1;
|
||||
}
|
||||
}
|
||||
|
||||
// Fall back to periodicity over the whole string (bounded by the same block
|
||||
// length). Find the smallest period that makes the SUFFIX highly repetitive.
|
||||
const n = text.length;
|
||||
const maxP = Math.min(MAX_PERIOD_LEN, Math.floor(n / MIN_PERIOD_REPEATS));
|
||||
for (let p = 1; p <= maxP; p++) {
|
||||
// Count how many trailing p-blocks are periodic.
|
||||
let reps = 1;
|
||||
let i = n - 1;
|
||||
for (; i >= p; i--) {
|
||||
if (text[i] !== text[i - p]) break;
|
||||
}
|
||||
// The loop above walks over the periodic suffix; its length is (n-1 - i).
|
||||
const periodicLen = n - 1 - i;
|
||||
reps = Math.floor(periodicLen / p) + 1;
|
||||
if (reps >= MIN_PERIOD_REPEATS) {
|
||||
const loopStart = n - reps * p; // start of the fully-periodic suffix
|
||||
const kept = text.slice(0, loopStart + keepRepeats * p);
|
||||
return kept + marker;
|
||||
}
|
||||
}
|
||||
return text;
|
||||
}
|
||||
@@ -232,16 +232,6 @@ describe('applyLoadTools (#332)', () => {
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
|
||||
});
|
||||
|
||||
it('loadTools description tells the model CORE tools are always active (#444)', () => {
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain(
|
||||
'Tools NOT listed in the catalog are CORE and ALWAYS active',
|
||||
);
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain('NEVER via loadTools');
|
||||
// Names it out explicitly so the model doesn't loadTools a core tool.
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain('createComment');
|
||||
expect(LOAD_TOOLS_DESCRIPTION).toContain('searchInPage');
|
||||
});
|
||||
});
|
||||
|
||||
describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {
|
||||
|
||||
@@ -84,10 +84,7 @@ export const LOAD_TOOLS_DESCRIPTION =
|
||||
'block in your instructions. Pass the EXACT tool names from the catalog; this\n' +
|
||||
'call only ACTIVATES them and returns { loaded: [...] } — the tools become\n' +
|
||||
'callable on your NEXT step. Load several names in one call when the task clearly\n' +
|
||||
'needs them. Unknown names are rejected with the list of valid ones.\n' +
|
||||
'Tools NOT listed in the catalog are CORE and ALWAYS active — call them directly,\n' +
|
||||
'NEVER via loadTools (e.g. createComment, listComments, resolveComment,\n' +
|
||||
'editPageText, searchInPage).';
|
||||
'needs them. Unknown names are rejected with the list of valid ones.';
|
||||
|
||||
/**
|
||||
* Tier + catalogLine for the INLINE ai-chat tools — those defined per-layer in
|
||||
|
||||
@@ -158,27 +158,4 @@ describe('EnvironmentService', () => {
|
||||
).toBe('https://app.example.com');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isAiChatFinalStepLockdownEnabled (#444)', () => {
|
||||
const build = (val?: string) =>
|
||||
new EnvironmentService({
|
||||
get: (key: string, def?: string) =>
|
||||
key === 'AI_CHAT_FINAL_STEP_LOCKDOWN' ? (val ?? def) : def,
|
||||
} as any);
|
||||
|
||||
it('defaults to OFF (false) when unset — the new anti-degeneration default', () => {
|
||||
expect(build(undefined).isAiChatFinalStepLockdownEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
it('is true only for the exact opt-in "true" (case-insensitive)', () => {
|
||||
expect(build('true').isAiChatFinalStepLockdownEnabled()).toBe(true);
|
||||
expect(build('TRUE').isAiChatFinalStepLockdownEnabled()).toBe(true);
|
||||
});
|
||||
|
||||
it('stays OFF for any other value', () => {
|
||||
expect(build('false').isAiChatFinalStepLockdownEnabled()).toBe(false);
|
||||
expect(build('1').isAiChatFinalStepLockdownEnabled()).toBe(false);
|
||||
expect(build('yes').isAiChatFinalStepLockdownEnabled()).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -292,24 +292,6 @@ export class EnvironmentService {
|
||||
return enabled === 'true';
|
||||
}
|
||||
|
||||
/**
|
||||
* Final-step lockdown for the in-app agent loop (#444). When ON (legacy), the
|
||||
* LAST allowed step forces a text-only answer: tools are stripped
|
||||
* (toolChoice:'none') and a synthesis instruction is appended. Defaults to OFF:
|
||||
* stripping the tools mid-work triggered a token-loop degeneration incident
|
||||
* (the model, robbed of its tools on the final step, emitted a 255KB block
|
||||
* repeating a single token). With the toggle OFF the last step keeps its tools
|
||||
* and gets only a SOFT nudge to finish with a text summary; the universal
|
||||
* anti-babble guard is the token-degeneration detector instead. Enable this
|
||||
* only for a model that does NOT reliably end its turns with a text answer.
|
||||
*/
|
||||
isAiChatFinalStepLockdownEnabled(): boolean {
|
||||
const enabled = this.configService
|
||||
.get<string>('AI_CHAT_FINAL_STEP_LOCKDOWN', 'false')
|
||||
.toLowerCase();
|
||||
return enabled === 'true';
|
||||
}
|
||||
|
||||
/**
|
||||
* Resumable SSE transport for durable agent runs (#184 phase 1.5). When
|
||||
* enabled, a run tees its SSE frames into the in-memory run-stream registry so
|
||||
|
||||
@@ -1,3 +1,8 @@
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import Fastify, { FastifyInstance } from 'fastify';
|
||||
import fastifyStatic from '@fastify/static';
|
||||
import { resolveStaticAssetHeaders } from './static.module';
|
||||
|
||||
// Unit tests for the static-asset cache classifier extracted from the
|
||||
@@ -33,3 +38,69 @@ describe('resolveStaticAssetHeaders', () => {
|
||||
expect(headers['vary']).toBe('Accept-Encoding');
|
||||
});
|
||||
});
|
||||
|
||||
// Integration test proving the ACTUAL response header emitted by @fastify/static
|
||||
// with the exact registration options StaticModule uses. This is the regression
|
||||
// guard for #452: without `cacheControl: false`, @fastify/static writes its own
|
||||
// `Cache-Control: public, max-age=0` AFTER the setHeaders callback, overwriting
|
||||
// the immutable header — the /assets/ assertion below would then fail.
|
||||
describe('static.module @fastify/static registration (integration)', () => {
|
||||
let app: FastifyInstance;
|
||||
let tmpDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'static-module-spec-'));
|
||||
fs.mkdirSync(join(tmpDir, 'assets'), { recursive: true });
|
||||
fs.mkdirSync(join(tmpDir, 'locales'), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
join(tmpDir, 'assets', 'index-a1b2c3.js'),
|
||||
'console.log(1);',
|
||||
);
|
||||
fs.writeFileSync(join(tmpDir, 'locales', 'en.json'), '{"hello":"world"}');
|
||||
|
||||
app = Fastify();
|
||||
// Mirror StaticModule.onModuleInit's registration options exactly.
|
||||
await app.register(fastifyStatic, {
|
||||
root: tmpDir,
|
||||
wildcard: false,
|
||||
preCompressed: true,
|
||||
cacheControl: false,
|
||||
setHeaders: (res, filePath) => {
|
||||
for (const [name, value] of Object.entries(
|
||||
resolveStaticAssetHeaders(filePath),
|
||||
)) {
|
||||
res.setHeader(name, value);
|
||||
}
|
||||
},
|
||||
});
|
||||
await app.ready();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await app.close();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('serves a hashed /assets/ file with an immutable, 1-year cache-control', async () => {
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: '/assets/index-a1b2c3.js',
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
const cacheControl = res.headers['cache-control'];
|
||||
expect(cacheControl).toContain('immutable');
|
||||
expect(cacheControl).toContain('max-age=31536000');
|
||||
});
|
||||
|
||||
it('serves a non-hashed /locales/ file WITHOUT an immutable cache-control', async () => {
|
||||
const res = await app.inject({ method: 'GET', url: '/locales/en.json' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
// resolveStaticAssetHeaders sets no cache-control here and cacheControl:false
|
||||
// stops @fastify/static from adding one, so the browser revalidates by
|
||||
// etag/last-modified — either an absent header or one without `immutable`.
|
||||
const cacheControl = res.headers['cache-control'];
|
||||
if (cacheControl !== undefined) {
|
||||
expect(cacheControl).not.toContain('immutable');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -115,6 +115,11 @@ export class StaticModule implements OnModuleInit {
|
||||
// Serve the build-time .br/.gz neighbour when the client accepts it
|
||||
// (see vite-plugin-compression2 in apps/client/vite.config.ts).
|
||||
preCompressed: true,
|
||||
// @fastify/static's default cacheControl:true writes its own
|
||||
// Cache-Control (from maxAge, default 0) AFTER the setHeaders callback,
|
||||
// silently overwriting the immutable header that resolveStaticAssetHeaders
|
||||
// sets — disable it so setHeaders/resolveStaticAssetHeaders own the header.
|
||||
cacheControl: false,
|
||||
setHeaders: (res, filePath) => {
|
||||
for (const [name, value] of Object.entries(
|
||||
resolveStaticAssetHeaders(filePath),
|
||||
|
||||
Reference in New Issue
Block a user