vvzvlad/gitmost
1
0
Fork 0
Code Issues 27 Pull Requests 16 Packages Projects Releases Activity

test(ai-chat): align #332 deferred-tool test with #490 cross-turn persistence

Browse Source 
The #332 integration test (ai-chat-stream.int-spec.ts) asserted a deferred
tool activated in one turn does NOT leak into the next ("cold start per
turn"). #490 (f5bbfdb2) deliberately reversed that: it persists the
activation set into chat metadata.activatedTools and seeds the next turn
from it, so the model need not re-run loadTools. The test only surfaced now
because the token-estimate CI fix let the integration step run again.

Adopt #490 persistence as the intended behavior:
- flip the turn-2 assertion to expect createPage IS active on the fresh
  turn's first step (seeded from metadata); keep turn-1 cold-start assertions.
- rewrite the test docstring, describe/it titles and comments accordingly.
- fix two stale "not persisted / per-turn" comments in ai-chat.service.ts
  (prepareAgentStep + the streaming-loop activation block); no logic change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
agent_vscode
2026-07-12 05:16:47 +03:00
parent 0b8497e496
commit a42f1ead48
2 changed files with 30 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/server/src/core/ai-chat/ai-chat.service.ts
+10 -8
View File
@@ -189,10 +189,11 @@ export function stepBudgetWarning(stepNumber: number): string {
//
// `system` is the in-scope system prompt; we CONCATENATE so the original
// persona/context is preserved — a bare `system` override would REPLACE the
// whole system prompt for the step. `activatedTools` is PER-TURN mutable state
// owned by the streaming loop (a closure Set grown by loadTools); it is passed
// in (not module-global, not persisted) so this stays a pure function of its
// arguments.
// whole system prompt for the step. `activatedTools` is a closure Set grown by
// loadTools and owned by the streaming loop; the caller seeds it from and
// persists it to the chat's metadata across turns (#490), but this function only
// READS the Set it is handed, so it stays a pure function of its arguments (not
// module-global).
//
// NOTE: at AI SDK v7 the per-step `system` field is renamed to `instructions`.
// On v6 (`^6.0.134`) `system` is the correct field — adjust when bumping.
@@ -1410,10 +1411,11 @@ export class AiChatService implements OnModuleInit, OnModuleDestroy {
const baseTools = { ...external.tools, ...docmostTools };
// Deferred tool loading state (#332), scoped to THIS streaming loop:
// - `activatedTools` is per-TURN mutable state — a fresh closure Set created
// per streamText call, NOT module-global and NOT persisted, so a new turn
// starts cold. loadTools.execute adds to it; prepareAgentStep reads it to
// widen `activeTools` on the NEXT step.
// - `activatedTools` is a fresh closure Set per streamText call (not
// module-global), SEEDED from the chat's persisted metadata.activatedTools
// (#490, just below) so activation carries across turns. loadTools.execute
// adds to it; prepareAgentStep reads it to widen `activeTools` on the NEXT
// step; turn end persists it back.
// - `validDeferredNames` = every tool that is NOT core (the in-app deferred
// tools + ALL external MCP tools), computed from the ACTUAL toolset so an
// external tool is loadable by its namespaced name. loadTools rejects any
apps/server/test/integration/ai-chat-stream.int-spec.ts
+20 -18
View File
@@ -322,20 +322,21 @@ describe('AiChatService.stream [integration]', () => {
});
/**
* #332 deferred tool loading, the ON path. The riskiest property is that the
* per-turn `activatedTools` Set is created FRESH inside each stream() call, so a
* tool a previous turn activated via loadTools is NOT still active when the next
* turn starts — the new turn begins "cold" (CORE + loadTools only). The unit
* tests only exercise pure prepareAgentStep with hand-fed Sets; this pins the
* real wiring end-to-end (loadTools.execute -> activatedTools -> prepareStep ->
* per-step activeTools) against the real streamText loop, and proves there is no
* cross-turn leak. We drive a MockLanguageModelV3 whose step 1 calls
* loadTools(['createPage']) and assert, via the model's recorded per-step
* CallOptions.tools (the AI SDK filters the provider tool list by activeTools),
* that the deferred tool becomes active on the SAME turn's next step but NOT on a
* fresh turn's first step.
* #332 + #490 deferred tool loading, the ON path. Turn 1 starts COLD (CORE +
* loadTools only) and activates a deferred tool via loadTools; that activation
* is PERSISTED into the chat's metadata.activatedTools (#490) so the NEXT turn
* SEEDS from it and the tool is active from the fresh turn's FIRST step — the
* model never re-runs loadTools to re-activate the same tool. The unit tests
* only exercise pure prepareAgentStep with hand-fed Sets; this pins the real
* wiring end-to-end (loadTools.execute -> activatedTools -> persist -> next-turn
* seed -> prepareStep -> per-step activeTools) against the real streamText loop.
* We drive a MockLanguageModelV3 whose step 1 calls loadTools(['createPage'])
* and assert, via the model's recorded per-step CallOptions.tools (the AI SDK
* filters the provider tool list by activeTools), that the deferred tool becomes
* active on the SAME turn's next step AND, seeded from metadata, on the next
* turn's first step.
*/
describe('deferred tool loading ON — per-turn activation, no leak (#332)', () => {
describe('deferred tool loading ON — cross-turn activation persistence (#332 + #490)', () => {
// A stub deferred (non-core) tool the agent can activate. Its execute is never
// called — the model only needs to SEE it become active — but it must be a
// valid AI-SDK tool so the SDK includes it in a step's tool list once active.
@@ -451,7 +452,7 @@ describe('AiChatService.stream [integration]', () => {
} as any);
}
it('activates a deferred tool for the SAME turn, and a NEW turn starts cold (no leak)', async () => {
it('activates a deferred tool for the SAME turn, and a NEW turn SEEDS it from persisted chat metadata (#490)', async () => {
const chatId = (await createChat(db, { workspaceId, creatorId: userId })).id;
// --- Turn 1: loadTools(createPage) on step 1, then answer on step 2. ---
@@ -474,7 +475,7 @@ describe('AiChatService.stream [integration]', () => {
// Step 2 of the SAME turn sees the just-activated deferred tool.
expect(step2Tools).toContain('createPage');
// --- Turn 2 on the SAME chat: must start cold again. ---
// --- Turn 2 on the SAME chat: seeds the persisted activation (#490). ---
const model2 = new MockLanguageModelV3({
doStream: async () => ({ stream: successStream() }),
} as any);
@@ -485,9 +486,10 @@ describe('AiChatService.stream [integration]', () => {
const nextTurnFirstStep = toolNames(model2.doStreamCalls[0]);
expect(nextTurnFirstStep).toContain('loadTools');
// The activated set is per-turn: the prior turn's createPage did NOT leak,
// so the fresh turn's first step sees it deferred again.
expect(nextTurnFirstStep).not.toContain('createPage');
// #490: activation PERSISTS across turns — turn 1 wrote createPage into the
// chat's metadata.activatedTools, so the next turn seeds from it and the
// deferred tool is active from the FIRST step (no need to re-run loadTools).
expect(nextTurnFirstStep).toContain('createPage');
});
});
});