From c2308949f854d660f7986a354fb0189a868221c6 Mon Sep 17 00:00:00 2001 From: Prasanth Date: Wed, 1 Jul 2026 15:58:04 +0530 Subject: [PATCH] refactor: extract conversation-history and turn-plan value objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent turn loop hand-rolled its transcript as a {role,content}[] and both run loops (Node + browser) duplicated the turn-mode derivation. Introduce two small, testable value objects the AttackRunner (PR8) will build on: - ConversationHistory (execute/conversationHistory.ts): wraps the transcript — push(user, assistant), messages (shallow snapshot), size, turnCount, and lastUser/lastAssistant for resume seeding. Adopted in runAgentLoop. - TurnPlan (execute/turnPlan.ts): from(config) resolves BOTH the default turn mode ("multi" when >1 turn, else "single") and the effective turn count ("single" forces one). Adopted in runAll and runAllBrowser, removing the duplicated derivation from both. Behavior-preserving: startTurn = turnCount + 1, resume seed guarded by turnCount >= 1, last-message lookups coalesce a missing content field to "" (as the old seeding did), judge still receives the transcript only past a single exchange. The orchestrator.equivalence and runAll.smoke guards stay green. Hardened after a max-effort review: TurnPlan absorbs the whole turn-mode derivation (not half), turnCount encapsulates the 2-messages-per-turn math, and the resume seed's null-coalesce is restored. A follow-up is logged to consolidate the several {role,content} synonyms into one canonical type. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/src/execute/conversationHistory.ts | 64 +++++++++++++++++++ core/src/execute/runAgentLoop.ts | 24 ++++--- core/src/execute/runAll.ts | 5 +- core/src/execute/runAllBrowser.ts | 4 +- core/src/execute/turnPlan.ts | 20 ++++++ core/tests/conversationHistory.test.ts | 84 +++++++++++++++++++++++++ core/tests/turnPlan.test.ts | 33 ++++++++++ 7 files changed, 216 insertions(+), 18 deletions(-) create mode 100644 core/src/execute/conversationHistory.ts create mode 100644 core/src/execute/turnPlan.ts create mode 100644 core/tests/conversationHistory.test.ts create mode 100644 core/tests/turnPlan.test.ts diff --git a/core/src/execute/conversationHistory.ts b/core/src/execute/conversationHistory.ts new file mode 100644 index 00000000..0935e008 --- /dev/null +++ b/core/src/execute/conversationHistory.ts @@ -0,0 +1,64 @@ +/** + * Value object for a multi-turn attack transcript. + * + * Wraps the hand-rolled `{ role, content }[]` the agent loop threaded by hand — + * appending a turn, seeding from a resumed run, and reporting size — behind a + * small, testable surface. The AttackRunner (PR8) builds on this instead of a + * bare array. Browser-safe: no Node imports. + */ +export interface ConversationMessage { + role: "user" | "assistant"; + content: string; +} + +export class ConversationHistory { + private readonly entries: ConversationMessage[]; + + constructor(initial: readonly ConversationMessage[] = []) { + this.entries = [...initial]; + } + + /** Append one user→assistant exchange (two messages). */ + push(userContent: string, assistantContent: string): void { + this.entries.push({ role: "user", content: userContent }); + this.entries.push({ role: "assistant", content: assistantContent }); + } + + /** + * A shallow copy of the message list for callers that consume a + * `{ role, content }[]`. The list is safe to reorder or extend; callers must + * not mutate the returned message objects in place. + */ + get messages(): ConversationMessage[] { + return this.entries.slice(); + } + + /** Total message count — two per completed turn. */ + get size(): number { + return this.entries.length; + } + + /** Completed user→assistant turns (two messages each). */ + get turnCount(): number { + return Math.floor(this.entries.length / 2); + } + + /** Content of the most recent user message, or "" if there is none. */ + lastUser(): string { + return this.lastContentOf("user"); + } + + /** Content of the most recent assistant message, or "" if there is none. */ + lastAssistant(): string { + return this.lastContentOf("assistant"); + } + + private lastContentOf(role: ConversationMessage["role"]): string { + for (let i = this.entries.length - 1; i >= 0; i--) { + // `?? ""` mirrors the old seeding, which coalesced a missing content field + // from a malformed resumed transcript to "" rather than undefined. + if (this.entries[i].role === role) return this.entries[i].content ?? ""; + } + return ""; + } +} diff --git a/core/src/execute/runAgentLoop.ts b/core/src/execute/runAgentLoop.ts index aa5090ac..7b3b53fa 100644 --- a/core/src/execute/runAgentLoop.ts +++ b/core/src/execute/runAgentLoop.ts @@ -17,6 +17,7 @@ import { log } from "../lib/logger.js"; import type { AgentAttackSpec, AttackResult, AgentTurnRecord } from "./types.js"; import type { TelemetryConfig } from "../config/types.js"; import type { UnifiedTargetConfig } from "./types.js"; +import { ConversationHistory } from "./conversationHistory.js"; export async function runAgentAttack( attack: AgentAttackSpec, @@ -33,9 +34,7 @@ export async function runAgentAttack( } ): Promise { const turns: AgentTurnRecord[] = []; - const history: { role: "user" | "assistant"; content: string }[] = [ - ...(context?.initialHistory ?? []), - ]; + const history = new ConversationHistory(context?.initialHistory); // Parallel meta channel for attacker tag output (not sent to target). // Used to thread PREVIOUS_TECHNIQUE into the next turn's user-block. // Resume limitation: on resumed runs (initialHistory non-empty), this @@ -48,10 +47,10 @@ export async function runAgentAttack( let finalPrompt = attack.prompt ?? ""; let finalResponse = ""; - // For resume: if we already have history, seed finalPrompt/Response from it - if (history.length >= 2) { - finalPrompt = [...history].reverse().find((m) => m.role === "user")?.content ?? ""; - finalResponse = [...history].reverse().find((m) => m.role === "assistant")?.content ?? ""; + // For resume: if we already have a completed turn, seed finalPrompt/Response from it + if (history.turnCount >= 1) { + finalPrompt = history.lastUser(); + finalResponse = history.lastAssistant(); } const propagation = context?.telemetry?.propagation; @@ -68,7 +67,7 @@ export async function runAgentAttack( // Agents that don't read sessionIdField just ignore it. const attackSessionId = randomUUID(); - const startTurn = Math.floor(history.length / 2) + 1; + const startTurn = history.turnCount + 1; for (let t = startTurn; t <= attack.turns; t++) { let prompt: string; // Mode discriminator. Comprehensive mode seeds `attack.prompt` from the @@ -82,7 +81,7 @@ export async function runAgentAttack( attackerMeta.push({}); } else { const result = await generateNextAdaptiveTurn({ - history, + history: history.messages, attack, patterns, target: context?.targetConfig ?? { @@ -113,14 +112,13 @@ export async function runAgentAttack( const response = await target.send(prompt, { sessionId: attackSessionId, - history, + history: history.messages, propagation, attackTraceId, attackIndex: Number.isFinite(Number(attackIndex)) ? Number(attackIndex) : undefined, }); - history.push({ role: "user", content: prompt }); - history.push({ role: "assistant", content: response }); + history.push(prompt, response); finalPrompt = prompt; finalResponse = response; @@ -151,7 +149,7 @@ export async function runAgentAttack( finalResponse, judgeModel, await buildJudgeObservability(context?.telemetry, attackTraceId, finalResponse), - history.length > 2 ? history : undefined, + history.size > 2 ? history.messages : undefined, { patternName: attack.patternName, judgeHint: attack.judgeHint }, attack.upstreamSessions ); diff --git a/core/src/execute/runAll.ts b/core/src/execute/runAll.ts index 4838fdcf..562438f6 100644 --- a/core/src/execute/runAll.ts +++ b/core/src/execute/runAll.ts @@ -40,6 +40,7 @@ import { getAdapter } from "../telemetry/adapter.js"; import { runSetupTraceCuration } from "../telemetry/curation.js"; import { log } from "../lib/logger.js"; import { isStopError, getStopReason } from "../lib/llmRetry.js"; +import { TurnPlan } from "./turnPlan.js"; export interface RunAllOptions { onProgress?: (event: ProgressEvent) => void; @@ -168,9 +169,7 @@ export async function runAll( log.info(`\n▶ ${evaluator.name} (${evaluator.id})`); } - const turnMode: "single" | "multi" = - config.turnMode ?? (config.turns > 1 ? "multi" : "single"); - const effectiveTurns = turnMode === "single" ? 1 : config.turns; + const { turnMode, effectiveTurns } = TurnPlan.from(config); let attacks: AttackSpec[]; try { diff --git a/core/src/execute/runAllBrowser.ts b/core/src/execute/runAllBrowser.ts index 9fda9c48..1d7d7b03 100644 --- a/core/src/execute/runAllBrowser.ts +++ b/core/src/execute/runAllBrowser.ts @@ -12,6 +12,7 @@ import { TargetStopError } from "../targets/agentTarget.js"; import { runAgentAttack } from "./runAgentLoop.js"; import { log } from "../lib/logger.js"; import { isStopError, getStopReason } from "../lib/llmRetry.js"; +import { TurnPlan } from "./turnPlan.js"; import { summarizeVerdicts, toEvaluatorResult, @@ -77,8 +78,7 @@ export async function runAllBrowser( notify({ type: "evaluator_start", evaluatorId: evaluator.id, evaluatorName: evaluator.name }); log.info(`\n▶ ${evaluator.name} (${evaluator.id})`); - const turnMode: "single" | "multi" = config.turnMode ?? (config.turns > 1 ? "multi" : "single"); - const effectiveTurns = turnMode === "single" ? 1 : config.turns; + const { turnMode, effectiveTurns } = TurnPlan.from(config); let generated; try { diff --git a/core/src/execute/turnPlan.ts b/core/src/execute/turnPlan.ts new file mode 100644 index 00000000..82bc26eb --- /dev/null +++ b/core/src/execute/turnPlan.ts @@ -0,0 +1,20 @@ +/** + * Value object for a run's turn shape. + * + * Owns the two-step rule the Node and browser run loops both derived by hand: + * 1. default the turn mode ("multi" when more than one turn is configured, else + * "single"), and + * 2. resolve the effective turn count ("single" forces one turn). + * Gives the AttackRunner (PR8) one testable place to ask for either. + */ +export class TurnPlan { + private constructor( + readonly turnMode: "single" | "multi", + readonly effectiveTurns: number + ) {} + + static from(config: { turnMode?: "single" | "multi"; turns: number }): TurnPlan { + const turnMode = config.turnMode ?? (config.turns > 1 ? "multi" : "single"); + return new TurnPlan(turnMode, turnMode === "single" ? 1 : config.turns); + } +} diff --git a/core/tests/conversationHistory.test.ts b/core/tests/conversationHistory.test.ts new file mode 100644 index 00000000..e913c2fc --- /dev/null +++ b/core/tests/conversationHistory.test.ts @@ -0,0 +1,84 @@ +/** + * PR7 — ConversationHistory value object. + * + * Pins the transcript behavior the agent loop relied on: seeding from a resumed + * run, appending turns, size, and last-message lookups. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { ConversationHistory } from "../src/execute/conversationHistory.js"; + +test("starts empty", () => { + const h = new ConversationHistory(); + assert.strictEqual(h.size, 0); + assert.deepStrictEqual(h.messages, []); + assert.strictEqual(h.lastUser(), ""); + assert.strictEqual(h.lastAssistant(), ""); +}); + +test("push appends a user→assistant exchange in order", () => { + const h = new ConversationHistory(); + h.push("u1", "a1"); + h.push("u2", "a2"); + assert.strictEqual(h.size, 4); + assert.deepStrictEqual(h.messages, [ + { role: "user", content: "u1" }, + { role: "assistant", content: "a1" }, + { role: "user", content: "u2" }, + { role: "assistant", content: "a2" }, + ]); +}); + +test("seeds from an initial (resumed) transcript without aliasing it", () => { + const initial = [ + { role: "user" as const, content: "u1" }, + { role: "assistant" as const, content: "a1" }, + ]; + const h = new ConversationHistory(initial); + assert.strictEqual(h.size, 2); + h.push("u2", "a2"); + // Mutating the history must not mutate the caller's array. + assert.strictEqual(initial.length, 2); +}); + +test("messages returns a snapshot copy, not the internal array", () => { + const h = new ConversationHistory(); + h.push("u1", "a1"); + const snapshot = h.messages; + snapshot.push({ role: "user", content: "injected" }); + assert.strictEqual(h.size, 2); // external mutation of the snapshot leaves state intact +}); + +test("turnCount reports completed user→assistant turns", () => { + const h = new ConversationHistory(); + assert.strictEqual(h.turnCount, 0); + h.push("u1", "a1"); + assert.strictEqual(h.turnCount, 1); + h.push("u2", "a2"); + assert.strictEqual(h.turnCount, 2); +}); + +test("last lookups coalesce a missing content field to '' (malformed resumed transcript)", () => { + // A malformed resumed transcript could carry an undefined content; the old + // seeding coalesced it to "" and the value object must preserve that. + const h = new ConversationHistory([ + { role: "user", content: undefined as unknown as string }, + { role: "assistant", content: undefined as unknown as string }, + ]); + assert.strictEqual(h.lastUser(), ""); + assert.strictEqual(h.lastAssistant(), ""); +}); + +test("lastUser / lastAssistant return the most recent of each role", () => { + const h = new ConversationHistory(); + h.push("u1", "a1"); + h.push("u2", "a2"); + assert.strictEqual(h.lastUser(), "u2"); + assert.strictEqual(h.lastAssistant(), "a2"); +}); + +test("last lookups return '' when the role is absent", () => { + const h = new ConversationHistory([{ role: "user", content: "only-user" }]); + assert.strictEqual(h.lastUser(), "only-user"); + assert.strictEqual(h.lastAssistant(), ""); +}); diff --git a/core/tests/turnPlan.test.ts b/core/tests/turnPlan.test.ts new file mode 100644 index 00000000..9094a571 --- /dev/null +++ b/core/tests/turnPlan.test.ts @@ -0,0 +1,33 @@ +/** + * PR7 — TurnPlan value object. + * + * Pins the turn-mode defaulting + single-vs-multi turn rule that was duplicated + * inline in runAll and runAllBrowser. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { TurnPlan } from "../src/execute/turnPlan.js"; + +test("explicit single turnMode forces one turn regardless of the configured count", () => { + const plan = TurnPlan.from({ turnMode: "single", turns: 5 }); + assert.strictEqual(plan.turnMode, "single"); + assert.strictEqual(plan.effectiveTurns, 1); +}); + +test("explicit multi turnMode honors the configured turn count", () => { + const plan = TurnPlan.from({ turnMode: "multi", turns: 3 }); + assert.strictEqual(plan.turnMode, "multi"); + assert.strictEqual(plan.effectiveTurns, 3); +}); + +test("omitted turnMode defaults to multi when more than one turn is configured", () => { + const plan = TurnPlan.from({ turns: 3 }); + assert.strictEqual(plan.turnMode, "multi"); + assert.strictEqual(plan.effectiveTurns, 3); +}); + +test("omitted turnMode defaults to single when one turn is configured", () => { + const plan = TurnPlan.from({ turns: 1 }); + assert.strictEqual(plan.turnMode, "single"); + assert.strictEqual(plan.effectiveTurns, 1); +});