From a53a9a0802fdee62e8cf3aa260b5f01d30b29b67 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 19:52:34 -0400 Subject: [PATCH 01/16] feat(engine): add SystemPromptLayers type and builder for cross-session caching --- .../src/__tests__/prompt-layers.test.ts | 125 ++++++++++++++++++ packages/engine/src/prompt-layers.ts | 81 ++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 packages/engine/src/__tests__/prompt-layers.test.ts create mode 100644 packages/engine/src/prompt-layers.ts diff --git a/packages/engine/src/__tests__/prompt-layers.test.ts b/packages/engine/src/__tests__/prompt-layers.test.ts new file mode 100644 index 000000000..d34e96c9e --- /dev/null +++ b/packages/engine/src/__tests__/prompt-layers.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect } from "vitest"; +import { + type SystemPromptLayers, + buildPromptLayers, + collapsePromptLayers, +} from "../prompt-layers.js"; + +describe("buildPromptLayers", () => { + it("separates base prompt into stable layer", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + }); + + expect(layers.stable).toBe("You are a reviewer."); + expect(layers.dynamic).toBe(""); + }); + + it("puts agent instructions into dynamic layer", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + agentInstructions: "Always check for SQL injection.", + }); + + expect(layers.stable).toBe("You are a reviewer."); + expect(layers.dynamic).toContain("Always check for SQL injection."); + }); + + it("puts memory section into dynamic layer", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + memorySection: "## Agent Memory\n\nRemember to check tests.", + }); + + expect(layers.stable).toBe("You are a reviewer."); + expect(layers.dynamic).toContain("Agent Memory"); + }); + + it("puts plugin contributions into dynamic layer", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + pluginContributions: "## Plugin: security\n\nScan for CVEs.", + }); + + expect(layers.stable).toBe("You are a reviewer."); + expect(layers.dynamic).toContain("security"); + }); + + it("puts performance feedback into dynamic layer", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + performanceFeedback: "## Performance Feedback\n\n- Average score: 8.5", + }); + + expect(layers.stable).toBe("You are a reviewer."); + expect(layers.dynamic).toContain("Performance Feedback"); + }); + + it("combines multiple dynamic sections with double newlines", () => { + const layers = buildPromptLayers({ + basePrompt: "Base.", + agentInstructions: "Instructions.", + memorySection: "Memory.", + pluginContributions: "Plugins.", + }); + + expect(layers.dynamic).toBe( + "## Custom Instructions\n\nInstructions.\n\nMemory.\n\nPlugins." + ); + }); + + it("omits empty dynamic sections", () => { + const layers = buildPromptLayers({ + basePrompt: "Base.", + agentInstructions: "", + memorySection: "", + pluginContributions: "Plugins.", + }); + + expect(layers.dynamic).toBe("Plugins."); + expect(layers.dynamic).not.toContain("Custom Instructions"); + }); + + it("produces deterministic output for identical inputs", () => { + const input = { + basePrompt: "Base.", + agentInstructions: "Inst.", + memorySection: "Mem.", + pluginContributions: "Plug.", + performanceFeedback: "Perf.", + }; + + const layers1 = buildPromptLayers(input); + const layers2 = buildPromptLayers(input); + + expect(layers1.stable).toBe(layers2.stable); + expect(layers1.dynamic).toBe(layers2.dynamic); + }); +}); + +describe("collapsePromptLayers", () => { + it("returns stable when dynamic is empty", () => { + const result = collapsePromptLayers({ stable: "Base.", dynamic: "" }); + expect(result).toBe("Base."); + }); + + it("joins stable and dynamic with double newline", () => { + const result = collapsePromptLayers({ + stable: "Base.", + dynamic: "Dynamic.", + }); + expect(result).toBe("Base.\n\nDynamic."); + }); + + it("matches legacy buildSystemPromptWithInstructions output", () => { + const layers = buildPromptLayers({ + basePrompt: "You are a reviewer.", + agentInstructions: "Check for bugs.", + }); + const collapsed = collapsePromptLayers(layers); + + expect(collapsed).toBe( + "You are a reviewer.\n\n## Custom Instructions\n\nCheck for bugs." + ); + }); +}); diff --git a/packages/engine/src/prompt-layers.ts b/packages/engine/src/prompt-layers.ts new file mode 100644 index 000000000..9e85e1c3b --- /dev/null +++ b/packages/engine/src/prompt-layers.ts @@ -0,0 +1,81 @@ +/** + * Structured system prompt layers for cross-session caching. + * + * The `stable` layer contains content that is identical across sessions of + * the same role (base role prompt). The `dynamic` layer holds per-session + * content (agent instructions, memory, performance feedback, plugins). + * + * When the stable layer is byte-identical across consecutive API calls, + * Anthropic's prompt cache gives a 90% read discount. OpenAI caches + * matching prefixes automatically at 50% discount. + */ +export interface SystemPromptLayers { + /** Role-specific base prompt — identical across all sessions of this role. */ + stable: string; + /** Per-session content: agent instructions, memory, feedback, plugins. */ + dynamic: string; +} + +export interface PromptLayerInput { + /** The base role system prompt (e.g. REVIEWER_SYSTEM_PROMPT). */ + basePrompt: string; + /** Resolved agent instructions (instructionsText + instructionsPath + soul). */ + agentInstructions?: string; + /** Formatted memory section (agent memory + workspace memory). */ + memorySection?: string; + /** Formatted plugin prompt contributions. */ + pluginContributions?: string; + /** Formatted performance feedback section. */ + performanceFeedback?: string; +} + +/** + * Build structured prompt layers from the components that currently get + * concatenated into a single system prompt string. + * + * The stable layer is ONLY the base role prompt. Everything else goes into + * the dynamic layer so that the stable prefix is byte-identical across + * sessions of the same role, enabling cross-session prompt caching. + */ +export function buildPromptLayers(input: PromptLayerInput): SystemPromptLayers { + const { basePrompt, agentInstructions, memorySection, pluginContributions, performanceFeedback } = input; + + const dynamicParts: string[] = []; + + const trimmedInstructions = agentInstructions?.trim() ?? ""; + if (trimmedInstructions) { + dynamicParts.push(`## Custom Instructions\n\n${trimmedInstructions}`); + } + + const trimmedMemory = memorySection?.trim() ?? ""; + if (trimmedMemory) { + dynamicParts.push(trimmedMemory); + } + + const trimmedPlugins = pluginContributions?.trim() ?? ""; + if (trimmedPlugins) { + dynamicParts.push(trimmedPlugins); + } + + const trimmedFeedback = performanceFeedback?.trim() ?? ""; + if (trimmedFeedback) { + dynamicParts.push(trimmedFeedback); + } + + return { + stable: basePrompt, + dynamic: dynamicParts.join("\n\n"), + }; +} + +/** + * Collapse layers back into a single string for backward compatibility. + * Runtimes that don't support structured caching use this to get the same + * concatenated prompt as before. + */ +export function collapsePromptLayers(layers: SystemPromptLayers): string { + if (!layers.dynamic) { + return layers.stable; + } + return `${layers.stable}\n\n${layers.dynamic}`; +} From e7ea1257ff4a5bdf6ec7712d989dc197873c3dda Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 19:54:55 -0400 Subject: [PATCH 02/16] feat(engine): add systemPromptLayers to AgentRuntimeOptions and wire into DefaultResourceLoader Extends AgentRuntimeOptions and AgentOptions with an optional systemPromptLayers field (SystemPromptLayers) for cross-session prompt caching. DefaultResourceLoader now uses the stable layer as systemPromptOverride and the dynamic layer as appendSystemPromptOverride when layers are provided, falling back to the flat systemPrompt string for backward compatibility. Co-Authored-By: Claude Sonnet 4.6 --- .../__tests__/agent-runtime-layers.test.ts | 31 +++++++++++ .../src/__tests__/pi-prompt-layers.test.ts | 53 +++++++++++++++++++ packages/engine/src/agent-runtime.ts | 12 +++++ packages/engine/src/pi.ts | 13 ++++- 4 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 packages/engine/src/__tests__/agent-runtime-layers.test.ts create mode 100644 packages/engine/src/__tests__/pi-prompt-layers.test.ts diff --git a/packages/engine/src/__tests__/agent-runtime-layers.test.ts b/packages/engine/src/__tests__/agent-runtime-layers.test.ts new file mode 100644 index 000000000..bad5e7ff1 --- /dev/null +++ b/packages/engine/src/__tests__/agent-runtime-layers.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from "vitest"; +import type { AgentRuntimeOptions } from "../agent-runtime.js"; +import type { SystemPromptLayers } from "../prompt-layers.js"; + +describe("AgentRuntimeOptions.systemPromptLayers", () => { + it("accepts systemPromptLayers alongside systemPrompt", () => { + const layers: SystemPromptLayers = { + stable: "You are a reviewer.", + dynamic: "Check for bugs.", + }; + + const options: AgentRuntimeOptions = { + cwd: "/tmp/test", + systemPrompt: "You are a reviewer.\n\nCheck for bugs.", + systemPromptLayers: layers, + }; + + expect(options.systemPromptLayers).toBeDefined(); + expect(options.systemPromptLayers!.stable).toBe("You are a reviewer."); + expect(options.systemPromptLayers!.dynamic).toBe("Check for bugs."); + }); + + it("works without systemPromptLayers (backward compatible)", () => { + const options: AgentRuntimeOptions = { + cwd: "/tmp/test", + systemPrompt: "You are a reviewer.", + }; + + expect(options.systemPromptLayers).toBeUndefined(); + }); +}); diff --git a/packages/engine/src/__tests__/pi-prompt-layers.test.ts b/packages/engine/src/__tests__/pi-prompt-layers.test.ts new file mode 100644 index 000000000..7b9e74e6f --- /dev/null +++ b/packages/engine/src/__tests__/pi-prompt-layers.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from "vitest"; + +describe("createFnAgent prompt layer configuration", () => { + it("uses stable layer as systemPromptOverride when layers provided", () => { + const options = { + systemPrompt: "Stable.\n\nDynamic.", + systemPromptLayers: { stable: "Stable.", dynamic: "Dynamic." }, + }; + + const systemPromptOverride = + options.systemPromptLayers?.stable ?? options.systemPrompt; + const appendSystemPromptOverride = options.systemPromptLayers?.dynamic + ? [options.systemPromptLayers.dynamic] + : []; + + expect(systemPromptOverride).toBe("Stable."); + expect(appendSystemPromptOverride).toEqual(["Dynamic."]); + }); + + it("falls back to full systemPrompt when layers not provided", () => { + const options = { + systemPrompt: "Full prompt.", + systemPromptLayers: undefined as + | { stable: string; dynamic: string } + | undefined, + }; + + const systemPromptOverride = + options.systemPromptLayers?.stable ?? options.systemPrompt; + const appendSystemPromptOverride = options.systemPromptLayers?.dynamic + ? [options.systemPromptLayers.dynamic] + : []; + + expect(systemPromptOverride).toBe("Full prompt."); + expect(appendSystemPromptOverride).toEqual([]); + }); + + it("handles empty dynamic layer", () => { + const options = { + systemPrompt: "Stable.", + systemPromptLayers: { stable: "Stable.", dynamic: "" }, + }; + + const systemPromptOverride = + options.systemPromptLayers?.stable ?? options.systemPrompt; + const appendSystemPromptOverride = options.systemPromptLayers?.dynamic + ? [options.systemPromptLayers.dynamic] + : []; + + expect(systemPromptOverride).toBe("Stable."); + expect(appendSystemPromptOverride).toEqual([]); + }); +}); diff --git a/packages/engine/src/agent-runtime.ts b/packages/engine/src/agent-runtime.ts index af8c2a4ab..9b378aba0 100644 --- a/packages/engine/src/agent-runtime.ts +++ b/packages/engine/src/agent-runtime.ts @@ -19,6 +19,7 @@ import type { PermanentAgentGatingContext } from "@fusion/core"; import type { SkillSelectionContext } from "./skill-resolver.js"; import type { FallbackModelUsedPayload } from "./pi.js"; import type { AgentActionGateContext } from "./agent-action-gate.js"; +import type { SystemPromptLayers } from "./prompt-layers.js"; /** * Options for creating an agent session. @@ -36,6 +37,17 @@ export interface AgentRuntimeOptions { cwd: string; /** System prompt for the agent */ systemPrompt: string; + /** + * Optional structured prompt layers for cross-session caching. + * When present, runtimes that support prompt caching use the `stable` + * layer as a cacheable prefix and the `dynamic` layer as the per-session + * suffix. Runtimes that don't support caching ignore this and use + * `systemPrompt` (the collapsed string) instead. + * + * Callers MUST also provide `systemPrompt` as the collapsed equivalent + * for backward compatibility. + */ + systemPromptLayers?: SystemPromptLayers; /** Tool set to use: "coding" for full tools, "readonly" for read-only access */ tools?: "coding" | "readonly"; /** Additional custom tools to merge with the base toolset */ diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 6dbb89184..d34686c23 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -56,6 +56,7 @@ import { type AgentActionGateContext, } from "./agent-action-gate.js"; import { resolvePermanentAgentToolDecision } from "./permanent-agent-gating.js"; +import type { SystemPromptLayers } from "./prompt-layers.js"; export interface AgentResult { session: AgentSession; @@ -711,6 +712,11 @@ export type BuiltinWebToolName = "WebSearch" | "WebFetch"; export interface AgentOptions { cwd: string; systemPrompt: string; + /** Structured prompt layers for cross-session caching. When provided, + * the stable layer is used as systemPromptOverride and the dynamic + * layer as appendSystemPromptOverride. Falls back to systemPrompt + * when not provided. */ + systemPromptLayers?: SystemPromptLayers; tools?: "coding" | "readonly"; customTools?: ToolDefinition[]; /** Optional allowlist of builtin runtime web tools to keep enabled. */ @@ -1715,8 +1721,11 @@ export async function createFnAgent(options: AgentOptions): Promise cwd: resolvedProjectRoot, agentDir: getFusionAgentDir(), settingsManager, - systemPromptOverride: () => options.systemPrompt, - appendSystemPromptOverride: () => [], + systemPromptOverride: () => options.systemPromptLayers?.stable ?? options.systemPrompt, + appendSystemPromptOverride: () => + options.systemPromptLayers?.dynamic + ? [options.systemPromptLayers.dynamic] + : [], ...(effectiveExtensionPaths.length > 0 ? { additionalExtensionPaths: [...effectiveExtensionPaths] } : {}), ...(skillsOverrideFn ? { skillsOverride: skillsOverrideFn } : {}), }); From 00bc7cd442c788e94ac089538cd94243960e140b Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 19:57:13 -0400 Subject: [PATCH 03/16] feat(engine): reviewer uses prompt layers for cross-session caching --- .../__tests__/reviewer-prompt-layers.test.ts | 45 +++++++++++++++++++ packages/engine/src/reviewer.ts | 33 ++++++++------ 2 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 packages/engine/src/__tests__/reviewer-prompt-layers.test.ts diff --git a/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts b/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts new file mode 100644 index 000000000..54de4cb16 --- /dev/null +++ b/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from "vitest"; +import { buildPromptLayers, collapsePromptLayers } from "../prompt-layers.js"; + +describe("reviewer prompt layering", () => { + const REVIEWER_BASE = "You are an independent code and plan reviewer."; + const MEMORY_INSTRUCTIONS = "\n## Memory\n\nUse fn_memory_search to look up context."; + + it("puts base prompt + memory instructions in stable layer", () => { + const layers = buildPromptLayers({ + basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + agentInstructions: "Custom reviewer guidance.", + pluginContributions: "## Plugin: lint\n\nCheck lint.", + }); + + expect(layers.stable).toBe(REVIEWER_BASE + MEMORY_INSTRUCTIONS); + expect(layers.stable).not.toContain("Custom reviewer guidance"); + expect(layers.stable).not.toContain("lint"); + }); + + it("produces identical stable layer across simulated sessions", () => { + const layers1 = buildPromptLayers({ + basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + agentInstructions: "Session 1 instructions.", + }); + const layers2 = buildPromptLayers({ + basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + agentInstructions: "Session 2 instructions.", + }); + + expect(layers1.stable).toBe(layers2.stable); + }); + + it("collapsed layers match legacy concatenation", () => { + const layers = buildPromptLayers({ + basePrompt: REVIEWER_BASE, + agentInstructions: "Check for bugs.", + pluginContributions: "## Plugin: sec\n\nScan.", + }); + const collapsed = collapsePromptLayers(layers); + + expect(collapsed).toBe( + `${REVIEWER_BASE}\n\n## Custom Instructions\n\nCheck for bugs.\n\n## Plugin: sec\n\nScan.` + ); + }); +}); diff --git a/packages/engine/src/reviewer.ts b/packages/engine/src/reviewer.ts index 415a1f91b..f0a2a473d 100644 --- a/packages/engine/src/reviewer.ts +++ b/packages/engine/src/reviewer.ts @@ -18,9 +18,9 @@ import { reviewerLog } from "./logger.js"; import { checkSessionError } from "./usage-limit-detector.js"; import { resolveAgentInstructions, - buildSystemPromptWithInstructions, buildPluginPromptSection, } from "./agent-instructions.js"; +import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js"; import { createFallbackModelObserver } from "./fallback-model-observer.js"; import { createMemoryGetTool, createMemorySearchTool, createWebFetchTool } from "./agent-tools.js"; @@ -408,23 +408,27 @@ export async function reviewStep( const memorySection = options.rootDir && options.settings?.memoryEnabled !== false ? "\n" + buildReviewerMemoryInstructions(options.rootDir, options.settings) : ""; - const reviewerSystemPrompt = buildSystemPromptWithInstructions( - reviewerBasePrompt + memorySection, - reviewerInstructions, - ); - const reviewerContributions = options.pluginRunner - ?.getPromptContributionsForSurface("reviewer") - ?? []; - if (reviewerContributions.length > 0) { - reviewerLog.log(`applied ${reviewerContributions.length} plugin prompt contributions for reviewer surface`); - } + + // Build structured layers for cross-session prompt caching. + // The stable layer (base prompt + memory instructions) is byte-identical + // across all reviewer sessions in this task, enabling cache hits. const reviewerPluginContributions = buildPluginPromptSection( "reviewer", options.pluginRunner, ); - const reviewerSystemPromptFinal = reviewerPluginContributions - ? `${reviewerSystemPrompt}\n\n${reviewerPluginContributions}` - : reviewerSystemPrompt; + if (reviewerPluginContributions) { + reviewerLog.log(`applied plugin prompt contributions for reviewer surface`); + } + + const layers = buildPromptLayers({ + basePrompt: reviewerBasePrompt + memorySection, + agentInstructions: reviewerInstructions, + pluginContributions: reviewerPluginContributions, + }); + + // Collapsed string for backward compatibility with runtimes that don't + // support layers (plugin runtimes, older pi versions). + const reviewerSystemPromptFinal = collapsePromptLayers(layers); // Build skill selection context (assigned agent skills take precedence over role fallback) let skillContext = undefined; @@ -495,6 +499,7 @@ export async function reviewStep( pluginRunner: options.pluginRunner, cwd, systemPrompt: reviewerSystemPromptFinal, + systemPromptLayers: layers, tools: "readonly", customTools: [createWebFetchTool(), ...(memoryTools ?? [])], onText: agentLogger ? agentLogger.onText : (delta) => options.onText?.(delta), From 39e57cbb88b69b1ddaf6901c82d162958c2ea5da Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 19:58:39 -0400 Subject: [PATCH 04/16] feat(engine): sort tools deterministically for prompt cache stability --- .../src/__tests__/tool-ordering.test.ts | 35 +++++++++++++++++++ packages/engine/src/pi.ts | 4 +++ 2 files changed, 39 insertions(+) create mode 100644 packages/engine/src/__tests__/tool-ordering.test.ts diff --git a/packages/engine/src/__tests__/tool-ordering.test.ts b/packages/engine/src/__tests__/tool-ordering.test.ts new file mode 100644 index 000000000..1a54d8ba5 --- /dev/null +++ b/packages/engine/src/__tests__/tool-ordering.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; + +/** + * Verifies that tools are sorted deterministically by name. + * This is critical for prompt caching — tool schemas are part of the + * API request, and reordering them breaks cache prefix matching. + */ +describe("deterministic tool ordering", () => { + it("sorts tools alphabetically by name", () => { + const tools = [ + { name: "write", execute: async () => {} }, + { name: "bash", execute: async () => {} }, + { name: "read", execute: async () => {} }, + { name: "edit", execute: async () => {} }, + ]; + + const sorted = [...tools].sort((a, b) => a.name.localeCompare(b.name)); + + expect(sorted.map((t) => t.name)).toEqual(["bash", "edit", "read", "write"]); + }); + + it("is stable across repeated sorts", () => { + const tools = [ + { name: "grep" }, + { name: "bash" }, + { name: "find" }, + { name: "read" }, + ]; + + const sorted1 = [...tools].sort((a, b) => a.name.localeCompare(b.name)); + const sorted2 = [...tools].sort((a, b) => a.name.localeCompare(b.name)); + + expect(sorted1.map((t) => t.name)).toEqual(sorted2.map((t) => t.name)); + }); +}); diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index d34686c23..06177eb41 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -1757,6 +1757,10 @@ export async function createFnAgent(options: AgentOptions): Promise boundaryContext.worktreePath, boundaryContext.worktreeProjectRoot, ); + // Sort tools alphabetically by name for deterministic ordering. + // Prompt caching requires the tool list to be byte-identical across + // sessions — reordering breaks cache prefix matching. + customToolList.sort((a, b) => a.name.localeCompare(b.name)); // Last-chance abort hook. Fires *here* — after every awaited setup step // in createFnAgent (provider registration, worktree validation, resource // loader reload) and immediately before the actual LLM session spawn. From 048a03b86b3c85670b4b78952219a8fa7e0322e7 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:00:08 -0400 Subject: [PATCH 05/16] feat(engine): scope skill loading by session purpose to reduce token overhead --- .../__tests__/skill-resolver-scoping.test.ts | 39 +++++++++++++++++ packages/engine/src/skill-resolver.ts | 43 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 packages/engine/src/__tests__/skill-resolver-scoping.test.ts diff --git a/packages/engine/src/__tests__/skill-resolver-scoping.test.ts b/packages/engine/src/__tests__/skill-resolver-scoping.test.ts new file mode 100644 index 000000000..f44d948df --- /dev/null +++ b/packages/engine/src/__tests__/skill-resolver-scoping.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from "vitest"; +import { getSkillPurposeFilter } from "../skill-resolver.js"; + +describe("getSkillPurposeFilter", () => { + it("returns a pass-all filter for executor sessions", () => { + const filter = getSkillPurposeFilter("executor"); + expect(filter("any-skill")).toBe(true); + expect(filter("fusion")).toBe(true); + expect(filter("deployment")).toBe(true); + }); + + it("filters to review-relevant skills for reviewer sessions", () => { + const filter = getSkillPurposeFilter("reviewer"); + expect(filter("code-review")).toBe(true); + expect(filter("security-review")).toBe(true); + expect(filter("review")).toBe(true); + expect(filter("deployment")).toBe(false); + expect(filter("fusion")).toBe(false); + }); + + it("returns a pass-all filter for unknown session purposes", () => { + const filter = getSkillPurposeFilter("unknown-purpose"); + expect(filter("any-skill")).toBe(true); + }); + + it("filters to minimal skills for heartbeat sessions", () => { + const filter = getSkillPurposeFilter("heartbeat"); + expect(filter("monitoring")).toBe(true); + expect(filter("heartbeat")).toBe(true); + expect(filter("fusion")).toBe(false); + expect(filter("code-review")).toBe(false); + }); + + it("returns a pass-all filter for triage sessions", () => { + const filter = getSkillPurposeFilter("triage"); + expect(filter("any-skill")).toBe(true); + expect(filter("fusion")).toBe(true); + }); +}); diff --git a/packages/engine/src/skill-resolver.ts b/packages/engine/src/skill-resolver.ts index 265d302ec..5c5669cb1 100644 --- a/packages/engine/src/skill-resolver.ts +++ b/packages/engine/src/skill-resolver.ts @@ -484,3 +484,46 @@ export function createSkillsOverrideFromSelection( }; }; } + +/** + * Skills that are relevant to each session purpose. Executors get all + * skills (they do the implementation work). Other roles get a scoped + * subset to avoid loading 71KB of reference material for every session. + * + * The filter matches against the skill's directory name (the last path + * segment before SKILL.md). + */ +const REVIEWER_SKILL_ALLOWLIST = new Set([ + "code-review", + "security-review", + "review", +]); + +const HEARTBEAT_SKILL_ALLOWLIST = new Set([ + "monitoring", + "heartbeat", +]); + +/** + * Return a filter function that decides whether a skill (by name) should + * be included in a session of the given purpose. + * + * - "executor" and "triage": all skills (pass-through) + * - "reviewer": only review-related skills + * - "heartbeat": only monitoring-related skills + * - unknown: all skills (safe fallback) + */ +export function getSkillPurposeFilter( + sessionPurpose: string, +): (skillName: string) => boolean { + switch (sessionPurpose) { + case "reviewer": + return (name) => REVIEWER_SKILL_ALLOWLIST.has(name); + case "heartbeat": + return (name) => HEARTBEAT_SKILL_ALLOWLIST.has(name); + case "executor": + case "triage": + default: + return () => true; + } +} From 4cebf1dde0150804822f2941fd4aaacd89c064de Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:01:14 -0400 Subject: [PATCH 06/16] feat(engine): add computeCacheHitRatio metric to token usage tracking --- .../__tests__/token-usage-cache-ratio.test.ts | 20 +++++++++++++++++++ packages/engine/src/session-token-usage.ts | 18 +++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 packages/engine/src/__tests__/token-usage-cache-ratio.test.ts diff --git a/packages/engine/src/__tests__/token-usage-cache-ratio.test.ts b/packages/engine/src/__tests__/token-usage-cache-ratio.test.ts new file mode 100644 index 000000000..5e0f1ec47 --- /dev/null +++ b/packages/engine/src/__tests__/token-usage-cache-ratio.test.ts @@ -0,0 +1,20 @@ +import { describe, it, expect } from "vitest"; +import { computeCacheHitRatio } from "../session-token-usage.js"; + +describe("computeCacheHitRatio", () => { + it("returns 0 when no tokens used", () => { + expect(computeCacheHitRatio(0, 0)).toBe(0); + }); + + it("returns 0 when no cached tokens", () => { + expect(computeCacheHitRatio(1000, 0)).toBe(0); + }); + + it("returns ratio of cached to total input", () => { + expect(computeCacheHitRatio(500, 500)).toBeCloseTo(0.5); + }); + + it("returns close to 1 when mostly cached", () => { + expect(computeCacheHitRatio(100, 9900)).toBeCloseTo(0.99); + }); +}); diff --git a/packages/engine/src/session-token-usage.ts b/packages/engine/src/session-token-usage.ts index ba7641172..3861fa4d4 100644 --- a/packages/engine/src/session-token-usage.ts +++ b/packages/engine/src/session-token-usage.ts @@ -90,3 +90,21 @@ export async function accumulateSessionTokenUsage( log.warn(`${taskId}: session token usage accumulate failed: ${message}`); } } + +/** + * Compute the cache hit ratio: the fraction of input tokens served from + * cache. Returns a number in [0, 1]. Useful for measuring the effectiveness + * of prompt caching optimizations. + * + * @param inputTokens - Non-cached input tokens (includes cache-write tokens) + * @param cachedTokens - Tokens read from cache + * @returns Cache hit ratio in [0, 1], or 0 if no tokens used + */ +export function computeCacheHitRatio( + inputTokens: number, + cachedTokens: number, +): number { + const total = inputTokens + cachedTokens; + if (total === 0) return 0; + return cachedTokens / total; +} From 5d3531e0bea5bee2b76941a50030ca7bec377b5f Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:03:41 -0400 Subject: [PATCH 07/16] feat(engine): executor uses prompt layers for cross-session caching --- packages/engine/src/executor.ts | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 68b06989a..278f0c954 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -53,6 +53,7 @@ import { buildSystemPromptWithInstructions, buildPluginPromptSection, } from "./agent-instructions.js"; +import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js"; import type { AgentReflectionService } from "./agent-reflection.js"; import { createRunAuditor, generateSyntheticRunId, type EngineRunContext } from "./run-audit.js"; import { evaluateSpecStaleness, getPromptPath } from "./spec-staleness.js"; @@ -3077,21 +3078,23 @@ export class TaskExecutor { // Resolve per-agent custom instructions for the executor role const executorInstructions = await this.resolveInstructionsForRole("executor"); - const executorSystemPrompt = buildSystemPromptWithInstructions( - getExecutorSystemPrompt(settings), - executorInstructions, - ); - const executorSystemContributions = this.options.pluginRunner?.getPromptContributionsForSurface("executor-system") ?? []; - if (executorSystemContributions.length > 0) { - executorLog.log(`${task.id}: applied ${executorSystemContributions.length} plugin prompt contributions for executor-system surface`); - } + + // Build structured layers for cross-session prompt caching. const executorPluginContributions = buildPluginPromptSection( "executor-system", this.options.pluginRunner, ); - const executorSystemPromptFinal = executorPluginContributions - ? `${executorSystemPrompt}\n\n${executorPluginContributions}` - : executorSystemPrompt; + if (executorPluginContributions) { + executorLog.log(`${task.id}: applied plugin prompt contributions for executor-system surface`); + } + + const executorLayers = buildPromptLayers({ + basePrompt: getExecutorSystemPrompt(settings), + agentInstructions: executorInstructions, + pluginContributions: executorPluginContributions, + }); + + const executorSystemPromptFinal = collapsePromptLayers(executorLayers); // sessionFile must be let because it's destructured alongside session which is reassigned // eslint-disable-next-line prefer-const @@ -3101,6 +3104,7 @@ export class TaskExecutor { pluginRunner: this.options.pluginRunner, cwd: worktreePath, systemPrompt: executorSystemPromptFinal, + systemPromptLayers: executorLayers, tools: "coding", customTools, onText: agentLogger.onText, @@ -3430,6 +3434,7 @@ export class TaskExecutor { pluginRunner: this.options.pluginRunner, cwd: worktreePath, systemPrompt: executorSystemPromptFinal, + systemPromptLayers: executorLayers, tools: "coding", customTools, onText: agentLogger.onText, From 9e5607b6c671975929ce8cc44fe6150e6f7a5613 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:06:20 -0400 Subject: [PATCH 08/16] feat(engine): triage and heartbeat use prompt layers for caching --- packages/engine/src/agent-heartbeat.ts | 28 +++++++++---------- packages/engine/src/triage.ts | 37 +++++++++++++------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/engine/src/agent-heartbeat.ts b/packages/engine/src/agent-heartbeat.ts index 6ea0ec775..5f01f7594 100644 --- a/packages/engine/src/agent-heartbeat.ts +++ b/packages/engine/src/agent-heartbeat.ts @@ -26,10 +26,10 @@ import { createTaskCreateTool, createTaskLogToolWithContext, createTaskDocumentW import { AgentLogger } from "./agent-logger.js"; import { resolveAgentInstructionsWithRatings, - buildSystemPromptWithInstructions, buildPluginPromptSection, resolveAgentHeartbeatProcedure, } from "./agent-instructions.js"; +import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js"; import { heartbeatLog, formatError } from "./logger.js"; import { createRunAuditor, type EngineRunContext } from "./run-audit.js"; import { promptWithFallback } from "./pi.js"; @@ -1838,23 +1838,22 @@ export class HeartbeatMonitor { } } - const systemPrompt = buildSystemPromptWithInstructions( - baseHeartbeatSystemPrompt, - [resolvedInstructionsForIdentity, memoryInstructions, selfImprovePrompt].filter((part) => part.trim()).join("\n\n"), - ); - const heartbeatContributions = this.pluginRunner - ?.getPromptContributionsForSurface("heartbeat") - ?? []; - if (heartbeatContributions.length > 0) { - heartbeatLog.log(`applied ${heartbeatContributions.length} plugin prompt contributions for heartbeat surface`); - } + // Build structured layers for cross-session prompt caching. const heartbeatPluginContributions = buildPluginPromptSection( "heartbeat", this.pluginRunner, ); - const systemPromptFinal = heartbeatPluginContributions - ? `${systemPrompt}\n\n${heartbeatPluginContributions}` - : systemPrompt; + if (heartbeatPluginContributions) { + heartbeatLog.log(`applied plugin prompt contributions for heartbeat surface`); + } + + const heartbeatLayers = buildPromptLayers({ + basePrompt: baseHeartbeatSystemPrompt, + agentInstructions: [resolvedInstructionsForIdentity, memoryInstructions, selfImprovePrompt].filter((part) => part.trim()).join("\n\n"), + pluginContributions: heartbeatPluginContributions, + }); + + const systemPromptFinal = collapsePromptLayers(heartbeatLayers); // fn_heartbeat_done must be the last tool in the array (stable terminal signal) heartbeatTools.push(heartbeatDoneTool); @@ -1948,6 +1947,7 @@ export class HeartbeatMonitor { pluginRunner: this.pluginRunner, cwd: rootDir, systemPrompt: systemPromptFinal, + systemPromptLayers: heartbeatLayers, tools: "coding", customTools: heartbeatTools, defaultProvider: heartbeatSessionModels.defaultProvider, diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index 1b544cc5d..2cc62e27f 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -30,9 +30,9 @@ import { AgentLogger } from "./agent-logger.js"; import { resolveAgentInstructions, resolveAgentInstructionsWithRatings, - buildSystemPromptWithInstructions, buildPluginPromptSection, } from "./agent-instructions.js"; +import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js"; import { createFallbackModelObserver } from "./fallback-model-observer.js"; import { planLog, reviewerLog, formatError } from "./logger.js"; import { @@ -1014,30 +1014,29 @@ export class TriageProcessor { const triageIdentitySection = assignedAgent ? `## Identity\n\nYou are ${assignedAgent.name}${assignedAgent.title?.trim() ? `, ${assignedAgent.title.trim()}` : ""} (agent ID: ${assignedAgent.id}, role: ${assignedAgent.role}).` : ""; - const triageSystemPrompt = buildSystemPromptWithInstructions( - resolveAgentPrompt("triage", settings.agentPrompts) + // Build structured layers for cross-session prompt caching. + const triagePluginContributions = buildPluginPromptSection( + "triage", + this.options.pluginRunner, + ); + if (triagePluginContributions) { + planLog.log(`${task.id}: applied plugin prompt contributions for triage surface`); + } + + const triageLayers = buildPromptLayers({ + basePrompt: resolveAgentPrompt("triage", settings.agentPrompts) || (isFast ? FAST_TRIAGE_SYSTEM_PROMPT : TRIAGE_SYSTEM_PROMPT), - [ + agentInstructions: [ triageIdentitySection, triageInstructions, isResearchToolSurfaceEnabled(settings) ? getResearchGuidanceForSurface("triage") : "", ].filter((section) => section.trim()).join("\n\n"), - ); - const triageContributions = this.options.pluginRunner - ?.getPromptContributionsForSurface("triage") - ?? []; - if (triageContributions.length > 0) { - planLog.log(`${task.id}: applied ${triageContributions.length} plugin prompt contributions for triage surface`); - } - const triagePluginContributions = buildPluginPromptSection( - "triage", - this.options.pluginRunner, - ); - const triageSystemPromptFinal = triagePluginContributions - ? `${triageSystemPrompt}\n\n${triagePluginContributions}` - : triageSystemPrompt; + pluginContributions: triagePluginContributions, + }); + + const triageSystemPromptFinal = collapsePromptLayers(triageLayers); // Build skill selection context (assigned agent skills take precedence over role fallback) const skillContext = await buildSessionSkillContext({ @@ -1054,6 +1053,7 @@ export class TriageProcessor { pluginRunner: this.options.pluginRunner, cwd: this.rootDir, systemPrompt: triageSystemPromptFinal, + systemPromptLayers: triageLayers, tools: "coding", customTools, onText: agentLogger.onText, @@ -1294,6 +1294,7 @@ export class TriageProcessor { pluginRunner: this.options.pluginRunner, cwd: this.rootDir, systemPrompt: triageSystemPromptFinal, + systemPromptLayers: triageLayers, tools: "coding", customTools, onText: agentLogger.onText, From 8570f955e8255b15f922a9ef38587ecae9e1556e Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:07:57 -0400 Subject: [PATCH 09/16] test(engine): verify layer forwarding and cross-session cache invariant --- .../prompt-cache-integration.test.ts | 48 +++++++++++++++++++ .../__tests__/session-helpers-layers.test.ts | 20 ++++++++ 2 files changed, 68 insertions(+) create mode 100644 packages/engine/src/__tests__/prompt-cache-integration.test.ts create mode 100644 packages/engine/src/__tests__/session-helpers-layers.test.ts diff --git a/packages/engine/src/__tests__/prompt-cache-integration.test.ts b/packages/engine/src/__tests__/prompt-cache-integration.test.ts new file mode 100644 index 000000000..694852cbd --- /dev/null +++ b/packages/engine/src/__tests__/prompt-cache-integration.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from "vitest"; +import { buildPromptLayers, collapsePromptLayers, type SystemPromptLayers } from "../prompt-layers.js"; +import { REVIEWER_SYSTEM_PROMPT } from "../reviewer.js"; + +describe("cross-session prompt cache integration", () => { + const MEMORY_INSTRUCTIONS = "\n## Memory\n\nUse fn_memory_search to look up relevant context."; + + function simulateReviewerSession(sessionIndex: number): SystemPromptLayers { + return buildPromptLayers({ + basePrompt: REVIEWER_SYSTEM_PROMPT + MEMORY_INSTRUCTIONS, + agentInstructions: `Session ${sessionIndex}: custom instructions that vary per agent.`, + pluginContributions: sessionIndex % 2 === 0 + ? "## Plugin: lint\n\nCheck lint rules." + : "", + }); + } + + it("produces byte-identical stable prefixes across 10 reviewer sessions", () => { + const sessions = Array.from({ length: 10 }, (_, i) => simulateReviewerSession(i)); + + const stablePrefix = sessions[0].stable; + for (let i = 1; i < sessions.length; i++) { + expect(sessions[i].stable).toBe(stablePrefix); + } + }); + + it("dynamic layers vary across sessions as expected", () => { + const sessions = Array.from({ length: 5 }, (_, i) => simulateReviewerSession(i)); + + const uniqueDynamics = new Set(sessions.map((s) => s.dynamic)); + expect(uniqueDynamics.size).toBeGreaterThan(1); + }); + + it("collapsed layers produce valid non-empty strings", () => { + const sessions = Array.from({ length: 5 }, (_, i) => simulateReviewerSession(i)); + + for (const session of sessions) { + const collapsed = collapsePromptLayers(session); + expect(collapsed.length).toBeGreaterThan(0); + expect(collapsed).toContain("independent code and plan reviewer"); + } + }); + + it("stable prefix starts with REVIEWER_SYSTEM_PROMPT", () => { + const layers = simulateReviewerSession(0); + expect(layers.stable.startsWith(REVIEWER_SYSTEM_PROMPT)).toBe(true); + }); +}); diff --git a/packages/engine/src/__tests__/session-helpers-layers.test.ts b/packages/engine/src/__tests__/session-helpers-layers.test.ts new file mode 100644 index 000000000..bf5e5cd79 --- /dev/null +++ b/packages/engine/src/__tests__/session-helpers-layers.test.ts @@ -0,0 +1,20 @@ +import { describe, it, expect } from "vitest"; +import type { ResolvedSessionOptions } from "../agent-session-helpers.js"; +import type { SystemPromptLayers } from "../prompt-layers.js"; + +describe("ResolvedSessionOptions layer forwarding", () => { + it("includes systemPromptLayers in the type", () => { + const layers: SystemPromptLayers = { + stable: "Stable prefix.", + dynamic: "Dynamic suffix.", + }; + + const options: Partial = { + systemPrompt: "Stable prefix.\n\nDynamic suffix.", + systemPromptLayers: layers, + }; + + expect(options.systemPromptLayers).toBeDefined(); + expect(options.systemPromptLayers!.stable).toBe("Stable prefix."); + }); +}); From 441191332b9e982d7f68c2d6cb5e0dab4846ea4f Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Fri, 8 May 2026 20:21:01 -0400 Subject: [PATCH 10/16] test(engine): improve coverage for prompt layers wiring, backward compat, and skill scoping Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/pi-layers-wiring.test.ts | 318 ++++++++++++++++++ .../prompt-layers-backward-compat.test.ts | 189 +++++++++++ .../skill-scoping-integration.test.ts | 104 ++++++ 3 files changed, 611 insertions(+) create mode 100644 packages/engine/src/__tests__/pi-layers-wiring.test.ts create mode 100644 packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts create mode 100644 packages/engine/src/__tests__/skill-scoping-integration.test.ts diff --git a/packages/engine/src/__tests__/pi-layers-wiring.test.ts b/packages/engine/src/__tests__/pi-layers-wiring.test.ts new file mode 100644 index 000000000..5eebfe2fb --- /dev/null +++ b/packages/engine/src/__tests__/pi-layers-wiring.test.ts @@ -0,0 +1,318 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { PathLike } from "node:fs"; + +/** + * Tests that createFnAgent correctly wires prompt layers to + * DefaultResourceLoader and that tools are sorted deterministically. + * + * These tests verify the actual subsystem wiring rather than testing + * the layer logic in isolation. + */ + +const createAgentSessionMock = vi.fn(); +const createCodingToolsMock = vi.fn(() => []); +const createReadOnlyToolsMock = vi.fn(() => []); +const createExtensionRuntimeMock = vi.fn(); +const discoverAndLoadExtensionsMock = vi.fn().mockResolvedValue({ + runtime: { pendingProviderRegistrations: [] }, + errors: [], +}); +const packageManagerResolveMock = vi.fn().mockResolvedValue({ extensions: [] }); +const findMock = vi.fn(); +const getAllMock = vi.fn(() => [] as any[]); +const registerProviderMock = vi.fn(); +const refreshMock = vi.fn(); +const settingsManagerInMemoryMock = vi.fn(() => ({ kind: "settings-manager" })); +const setFallbackResolverMock = vi.fn(); +const reloadMock = vi.fn(async () => {}); +const execSyncMock = vi.fn((_cmd?: any, _opts?: any) => ""); +const existsSyncMock = vi.fn((_path: PathLike) => false); +const readFileSyncMock = vi.fn((_path?: any) => "{}"); +const readCustomProvidersMock = vi.fn(() => []); + +// Capture DefaultResourceLoader constructor args +let capturedResourceLoaderOptions: any = null; + +vi.mock("node:child_process", () => { + const execSyncFn = execSyncMock; + const kPromisifyCustom = Symbol.for("nodejs.util.promisify.custom"); + + const execFn: any = vi.fn((cmd: string, opts: any, cb: any) => { + const callback = typeof opts === "function" ? opts : cb; + const options = typeof opts === "function" ? {} : (opts ?? {}); + try { + const out = execSyncFn(cmd, { ...options, stdio: ["pipe", "pipe", "pipe"] }); + const stdout = out === undefined ? "" : out.toString(); + if (typeof callback === "function") callback(null, stdout, ""); + } catch (err) { + if (typeof callback === "function") { + const error = err as { stdout?: string; stderr?: string }; + callback(err, error?.stdout?.toString?.() ?? "", error?.stderr?.toString?.() ?? ""); + } + } + }); + + execFn[kPromisifyCustom] = (cmd: string, opts?: any) => + new Promise((resolve, reject) => { + execFn(cmd, opts, (err: any, stdout: string, stderr: string) => { + if (err) { + (err as Record).stdout = stdout; + (err as Record).stderr = stderr; + reject(err); + } else { + resolve({ stdout, stderr }); + } + }); + }); + return { execSync: execSyncFn, exec: execFn }; +}); + +vi.mock("node:fs", async () => { + const actual = await vi.importActual("node:fs"); + return { + ...actual, + existsSync: existsSyncMock, + readFileSync: readFileSyncMock, + }; +}); + +vi.mock("../custom-providers.js", () => ({ + readCustomProviders: readCustomProvidersMock, +})); + +vi.mock("@mariozechner/pi-coding-agent", () => ({ + AuthStorage: { + create: () => ({ + setFallbackResolver: setFallbackResolverMock, + }), + }, + createAgentSession: createAgentSessionMock, + createBashTool: () => ({ name: "bash" }), + createCodingTools: createCodingToolsMock, + createEditTool: () => ({ name: "edit" }), + createExtensionRuntime: createExtensionRuntimeMock, + createFindTool: () => ({ name: "find" }), + createGrepTool: () => ({ name: "grep" }), + createLsTool: () => ({ name: "ls" }), + createReadOnlyTools: createReadOnlyToolsMock, + createReadTool: () => ({ name: "read" }), + createWriteTool: () => ({ name: "write" }), + DefaultResourceLoader: class { + constructor(options: any) { + capturedResourceLoaderOptions = options; + } + async reload() { + await reloadMock(); + } + }, + DefaultPackageManager: class { + async resolve() { + return packageManagerResolveMock(); + } + }, + discoverAndLoadExtensions: discoverAndLoadExtensionsMock, + getAgentDir: () => "/mock-agent-dir", + ModelRegistry: class { + static create(..._args: unknown[]) { + return new (this as unknown as new () => unknown)(); + } + find(provider: string, modelId: string) { + return findMock(provider, modelId); + } + getAll() { + return getAllMock(); + } + registerProvider(name: string, config: unknown) { + return registerProviderMock(name, config); + } + refresh() { + return refreshMock(); + } + }, + SessionManager: { + inMemory: () => ({ kind: "session-manager" }), + }, + SettingsManager: { + create: vi.fn(), + inMemory: settingsManagerInMemoryMock, + }, +})); + +describe("createFnAgent prompt layer wiring", () => { + beforeEach(() => { + vi.clearAllMocks(); + capturedResourceLoaderOptions = null; + execSyncMock.mockReturnValue(""); + existsSyncMock.mockReturnValue(false); + readFileSyncMock.mockReturnValue("{}"); + readCustomProvidersMock.mockReturnValue([]); + findMock.mockImplementation((provider: string, modelId: string) => ({ provider, id: modelId })); + createAgentSessionMock.mockResolvedValue({ + session: { + prompt: vi.fn(), + subscribe: vi.fn(), + dispose: vi.fn(), + setThinkingLevel: vi.fn(), + }, + }); + }); + + it("passes stable layer as systemPromptOverride when layers provided", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Stable prefix.\n\nDynamic suffix.", + systemPromptLayers: { + stable: "Stable prefix.", + dynamic: "Dynamic suffix.", + }, + }); + + expect(capturedResourceLoaderOptions).toBeDefined(); + const override = capturedResourceLoaderOptions.systemPromptOverride(); + expect(override).toBe("Stable prefix."); + }); + + it("passes dynamic layer via appendSystemPromptOverride when layers provided", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Stable.\n\nDynamic content.", + systemPromptLayers: { + stable: "Stable.", + dynamic: "Dynamic content.", + }, + }); + + expect(capturedResourceLoaderOptions).toBeDefined(); + const appended = capturedResourceLoaderOptions.appendSystemPromptOverride(); + expect(appended).toEqual(["Dynamic content."]); + }); + + it("falls back to full systemPrompt when no layers provided", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Full system prompt.", + }); + + expect(capturedResourceLoaderOptions).toBeDefined(); + const override = capturedResourceLoaderOptions.systemPromptOverride(); + expect(override).toBe("Full system prompt."); + }); + + it("returns empty array from appendSystemPromptOverride when no layers", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Full prompt.", + }); + + const appended = capturedResourceLoaderOptions.appendSystemPromptOverride(); + expect(appended).toEqual([]); + }); + + it("returns empty array from appendSystemPromptOverride when dynamic is empty", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Stable only.", + systemPromptLayers: { + stable: "Stable only.", + dynamic: "", + }, + }); + + const appended = capturedResourceLoaderOptions.appendSystemPromptOverride(); + expect(appended).toEqual([]); + }); +}); + +describe("createFnAgent deterministic tool ordering", () => { + beforeEach(() => { + vi.clearAllMocks(); + capturedResourceLoaderOptions = null; + execSyncMock.mockReturnValue(""); + existsSyncMock.mockReturnValue(false); + readFileSyncMock.mockReturnValue("{}"); + readCustomProvidersMock.mockReturnValue([]); + findMock.mockImplementation((provider: string, modelId: string) => ({ provider, id: modelId })); + createAgentSessionMock.mockResolvedValue({ + session: { + prompt: vi.fn(), + subscribe: vi.fn(), + dispose: vi.fn(), + setThinkingLevel: vi.fn(), + }, + }); + }); + + it("passes tools to createAgentSession in alphabetical order", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Test.", + tools: "coding", + }); + + expect(createAgentSessionMock).toHaveBeenCalled(); + + const callArgs = createAgentSessionMock.mock.calls[0][0]; + const toolNames = (callArgs.customTools ?? []).map((t: any) => t.name); + + // Tools should be in alphabetical order + const sorted = [...toolNames].sort(); + expect(toolNames).toEqual(sorted); + }); + + it("sorts custom tools mixed with built-in tools", async () => { + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Test.", + tools: "coding", + customTools: [ + { name: "zzz_custom", execute: vi.fn() } as any, + { name: "aaa_custom", execute: vi.fn() } as any, + ], + }); + + const callArgs = createAgentSessionMock.mock.calls[0][0]; + const toolNames = (callArgs.customTools ?? []).map((t: any) => t.name); + + const sorted = [...toolNames].sort(); + expect(toolNames).toEqual(sorted); + }); + + it("sorts readonly tools with custom tools", async () => { + createReadOnlyToolsMock.mockReturnValueOnce([ + { name: "read" }, + { name: "grep" }, + { name: "find" }, + ] as any); + + const { createFnAgent } = await import("../pi.js"); + + await createFnAgent({ + cwd: "/tmp/test-project", + systemPrompt: "Test.", + tools: "readonly", + customTools: [ + { name: "fn_task_list", execute: vi.fn() } as any, + ], + }); + + const callArgs = createAgentSessionMock.mock.calls[0][0]; + const toolNames = (callArgs.customTools ?? []).map((t: any) => t.name); + + const sorted = [...toolNames].sort(); + expect(toolNames).toEqual(sorted); + }); +}); diff --git a/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts new file mode 100644 index 000000000..000ee086a --- /dev/null +++ b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts @@ -0,0 +1,189 @@ +import { describe, it, expect } from "vitest"; +import { buildSystemPromptWithInstructions } from "../agent-instructions.js"; +import { buildPromptLayers, collapsePromptLayers } from "../prompt-layers.js"; + +/** + * Backward compatibility tests: verify that the new layered prompt approach + * produces byte-identical output to the legacy buildSystemPromptWithInstructions + * + manual concatenation pattern used by each subsystem. + */ +describe("prompt layers backward compatibility", () => { + describe("collapsed layers match buildSystemPromptWithInstructions", () => { + it("matches when only base prompt is provided", () => { + const basePrompt = "You are a reviewer."; + + const oldResult = buildSystemPromptWithInstructions(basePrompt, ""); + const layers = buildPromptLayers({ basePrompt }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + + it("matches with base prompt and instructions", () => { + const basePrompt = "You are a reviewer."; + const instructions = "Check for SQL injection."; + + const oldResult = buildSystemPromptWithInstructions(basePrompt, instructions); + const layers = buildPromptLayers({ + basePrompt, + agentInstructions: instructions, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + + it("matches with base prompt, instructions, and plugin contributions", () => { + const basePrompt = "You are an executor."; + const instructions = "Follow TDD."; + const plugins = "## Plugin: security\n\nScan for vulnerabilities."; + + // Old approach: buildSystemPromptWithInstructions + manual concatenation + const oldSystemPrompt = buildSystemPromptWithInstructions(basePrompt, instructions); + const oldResult = `${oldSystemPrompt}\n\n${plugins}`; + + // New approach: buildPromptLayers + collapsePromptLayers + const layers = buildPromptLayers({ + basePrompt, + agentInstructions: instructions, + pluginContributions: plugins, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + + it("matches with empty instructions and plugin contributions", () => { + const basePrompt = "You are a triage agent."; + const plugins = "## Plugin: research\n\nUse web search."; + + // Old approach: buildSystemPromptWithInstructions returns base (empty instructions) + // then plugins appended + const oldSystemPrompt = buildSystemPromptWithInstructions(basePrompt, ""); + const oldResult = `${oldSystemPrompt}\n\n${plugins}`; + + const layers = buildPromptLayers({ + basePrompt, + pluginContributions: plugins, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + + it("matches with empty instructions and no plugins", () => { + const basePrompt = "You are a heartbeat agent."; + + const oldResult = buildSystemPromptWithInstructions(basePrompt, ""); + const layers = buildPromptLayers({ basePrompt }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + }); + + describe("reviewer assembly pattern", () => { + it("reproduces the reviewer prompt assembly", () => { + const basePrompt = "You are an independent code and plan reviewer."; + const memoryInstructions = "\n## Memory\n\nUse fn_memory_search."; + const agentInstructions = "Focus on security."; + const plugins = "## Plugin: lint\n\nCheck eslint."; + + // Old reviewer pattern: + // 1. buildSystemPromptWithInstructions(base + memory, instructions) + // 2. if plugins: concatenate + const oldSystemPrompt = buildSystemPromptWithInstructions( + basePrompt + memoryInstructions, + agentInstructions, + ); + const oldResult = `${oldSystemPrompt}\n\n${plugins}`; + + // New reviewer pattern: + const layers = buildPromptLayers({ + basePrompt: basePrompt + memoryInstructions, + agentInstructions, + pluginContributions: plugins, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + }); + + describe("executor assembly pattern", () => { + it("reproduces the executor prompt assembly", () => { + const basePrompt = "You are a task execution agent."; + const agentInstructions = "Follow the spec precisely."; + const plugins = "## Plugin: deploy\n\nCheck CI status."; + + // Old executor pattern: + // 1. buildSystemPromptWithInstructions(base, instructions) + // 2. if plugins: concatenate + const oldSystemPrompt = buildSystemPromptWithInstructions( + basePrompt, + agentInstructions, + ); + const oldResult = `${oldSystemPrompt}\n\n${plugins}`; + + const layers = buildPromptLayers({ + basePrompt, + agentInstructions, + pluginContributions: plugins, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + }); + + describe("heartbeat assembly pattern", () => { + it("reproduces the heartbeat prompt assembly with multi-part instructions", () => { + const basePrompt = "You are a heartbeat agent."; + const identitySection = "## Identity\n\nYou are Agent-1."; + const memoryInstructions = "## Memory\n\nUse memory tools."; + const selfImprovePrompt = "## Self-Improvement\n\nReview your performance."; + + // Old heartbeat pattern: + // 1. Join identity + memory + selfImprove with \n\n + // 2. buildSystemPromptWithInstructions(base, joined) + // 3. if plugins: concatenate + const joinedInstructions = [identitySection, memoryInstructions, selfImprovePrompt] + .filter((part) => part.trim()) + .join("\n\n"); + const oldResult = buildSystemPromptWithInstructions(basePrompt, joinedInstructions); + + const layers = buildPromptLayers({ + basePrompt, + agentInstructions: joinedInstructions, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + }); + + describe("triage assembly pattern", () => { + it("reproduces the triage prompt assembly with multi-part instructions", () => { + const basePrompt = "You are a task specification agent."; + const identitySection = "## Identity\n\nYou are TriageBot."; + const triageInstructions = "Be thorough."; + const researchGuidance = "## Research\n\nUse web search when needed."; + + // Old triage pattern: + // 1. Join identity + instructions + research with \n\n + // 2. buildSystemPromptWithInstructions(base, joined) + const joinedInstructions = [identitySection, triageInstructions, researchGuidance] + .filter((section) => section.trim()) + .join("\n\n"); + const oldResult = buildSystemPromptWithInstructions(basePrompt, joinedInstructions); + + const layers = buildPromptLayers({ + basePrompt, + agentInstructions: joinedInstructions, + }); + const newResult = collapsePromptLayers(layers); + + expect(newResult).toBe(oldResult); + }); + }); +}); diff --git a/packages/engine/src/__tests__/skill-scoping-integration.test.ts b/packages/engine/src/__tests__/skill-scoping-integration.test.ts new file mode 100644 index 000000000..8aeafe1cb --- /dev/null +++ b/packages/engine/src/__tests__/skill-scoping-integration.test.ts @@ -0,0 +1,104 @@ +import { describe, it, expect } from "vitest"; +import { getSkillPurposeFilter } from "../skill-resolver.js"; + +/** + * Integration-style tests for getSkillPurposeFilter that verify filtering + * against a realistic set of skill names typical of a Fusion project. + * + * The existing skill-resolver-scoping.test.ts tests individual skill names + * in isolation. These tests verify the aggregate filtering behavior: how + * many skills each purpose loads from a full project skill set, and that + * the filter produces sensible subsets. + */ +describe("getSkillPurposeFilter integration", () => { + // Simulate a realistic set of skill names from a Fusion project + const ALL_SKILLS = [ + "fusion", + "code-review", + "security-review", + "review", + "deployment", + "task-management", + "monitoring", + "heartbeat", + "research", + "web-search", + ]; + + it("executor loads all skills", () => { + const filter = getSkillPurposeFilter("executor"); + const loaded = ALL_SKILLS.filter(filter); + expect(loaded).toEqual(ALL_SKILLS); + }); + + it("reviewer loads only review-related skills", () => { + const filter = getSkillPurposeFilter("reviewer"); + const loaded = ALL_SKILLS.filter(filter); + expect(loaded).toEqual(["code-review", "security-review", "review"]); + expect(loaded).not.toContain("fusion"); + expect(loaded).not.toContain("deployment"); + expect(loaded).not.toContain("monitoring"); + }); + + it("heartbeat loads only monitoring-related skills", () => { + const filter = getSkillPurposeFilter("heartbeat"); + const loaded = ALL_SKILLS.filter(filter); + expect(loaded).toEqual(["monitoring", "heartbeat"]); + expect(loaded).not.toContain("fusion"); + expect(loaded).not.toContain("code-review"); + }); + + it("triage loads all skills (same as executor)", () => { + const filter = getSkillPurposeFilter("triage"); + const loaded = ALL_SKILLS.filter(filter); + expect(loaded).toEqual(ALL_SKILLS); + }); + + it("reviewer filters out the majority of skills", () => { + const filter = getSkillPurposeFilter("reviewer"); + const loaded = ALL_SKILLS.filter(filter); + // Reviewer should load significantly fewer skills than total + expect(loaded.length).toBeLessThan(ALL_SKILLS.length / 2); + }); + + it("heartbeat filters to a minimal subset", () => { + const filter = getSkillPurposeFilter("heartbeat"); + const loaded = ALL_SKILLS.filter(filter); + // Heartbeat should have even fewer than reviewer + expect(loaded.length).toBeLessThanOrEqual(2); + }); + + it("unknown purpose passes all skills through (safe fallback)", () => { + const filter = getSkillPurposeFilter("some-future-purpose"); + const loaded = ALL_SKILLS.filter(filter); + expect(loaded).toEqual(ALL_SKILLS); + }); + + it("reviewer and heartbeat produce disjoint sets from the same input", () => { + const reviewerFilter = getSkillPurposeFilter("reviewer"); + const heartbeatFilter = getSkillPurposeFilter("heartbeat"); + const reviewerSkills = ALL_SKILLS.filter(reviewerFilter); + const heartbeatSkills = ALL_SKILLS.filter(heartbeatFilter); + + // No skill should appear in both reviewer and heartbeat sets + const overlap = reviewerSkills.filter((s) => heartbeatSkills.includes(s)); + expect(overlap).toEqual([]); + }); + + it("executor is a superset of all other purpose filters", () => { + const executorFilter = getSkillPurposeFilter("executor"); + const reviewerFilter = getSkillPurposeFilter("reviewer"); + const heartbeatFilter = getSkillPurposeFilter("heartbeat"); + + const executorSkills = new Set(ALL_SKILLS.filter(executorFilter)); + const reviewerSkills = ALL_SKILLS.filter(reviewerFilter); + const heartbeatSkills = ALL_SKILLS.filter(heartbeatFilter); + + for (const skill of reviewerSkills) { + expect(executorSkills.has(skill)).toBe(true); + } + for (const skill of heartbeatSkills) { + expect(executorSkills.has(skill)).toBe(true); + } + }); +}); From 0bf9b9193c03a6ca0a73459e1a7485cebb0ad3e2 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 12:33:44 -0400 Subject: [PATCH 11/16] =?UTF-8?q?fix(engine):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20remove=20dead=20skill=20scoping,=20fix=20memory=20l?= =?UTF-8?q?ayer=20placement,=20clarify=20JSDoc,=20sort=20builtin=20allowli?= =?UTF-8?q?st,=20remove=20redundant=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/__tests__/pi-prompt-layers.test.ts | 53 --------- .../prompt-cache-integration.test.ts | 3 +- .../prompt-layers-backward-compat.test.ts | 27 +++-- .../__tests__/reviewer-prompt-layers.test.ts | 18 +-- .../__tests__/session-helpers-layers.test.ts | 20 ---- .../__tests__/skill-resolver-scoping.test.ts | 39 ------- .../skill-scoping-integration.test.ts | 104 ------------------ .../src/__tests__/tool-ordering.test.ts | 35 ------ packages/engine/src/pi.ts | 2 +- packages/engine/src/reviewer.ts | 8 +- packages/engine/src/session-token-usage.ts | 14 ++- packages/engine/src/skill-resolver.ts | 42 ------- 12 files changed, 40 insertions(+), 325 deletions(-) delete mode 100644 packages/engine/src/__tests__/pi-prompt-layers.test.ts delete mode 100644 packages/engine/src/__tests__/session-helpers-layers.test.ts delete mode 100644 packages/engine/src/__tests__/skill-resolver-scoping.test.ts delete mode 100644 packages/engine/src/__tests__/skill-scoping-integration.test.ts delete mode 100644 packages/engine/src/__tests__/tool-ordering.test.ts diff --git a/packages/engine/src/__tests__/pi-prompt-layers.test.ts b/packages/engine/src/__tests__/pi-prompt-layers.test.ts deleted file mode 100644 index 7b9e74e6f..000000000 --- a/packages/engine/src/__tests__/pi-prompt-layers.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { describe, it, expect } from "vitest"; - -describe("createFnAgent prompt layer configuration", () => { - it("uses stable layer as systemPromptOverride when layers provided", () => { - const options = { - systemPrompt: "Stable.\n\nDynamic.", - systemPromptLayers: { stable: "Stable.", dynamic: "Dynamic." }, - }; - - const systemPromptOverride = - options.systemPromptLayers?.stable ?? options.systemPrompt; - const appendSystemPromptOverride = options.systemPromptLayers?.dynamic - ? [options.systemPromptLayers.dynamic] - : []; - - expect(systemPromptOverride).toBe("Stable."); - expect(appendSystemPromptOverride).toEqual(["Dynamic."]); - }); - - it("falls back to full systemPrompt when layers not provided", () => { - const options = { - systemPrompt: "Full prompt.", - systemPromptLayers: undefined as - | { stable: string; dynamic: string } - | undefined, - }; - - const systemPromptOverride = - options.systemPromptLayers?.stable ?? options.systemPrompt; - const appendSystemPromptOverride = options.systemPromptLayers?.dynamic - ? [options.systemPromptLayers.dynamic] - : []; - - expect(systemPromptOverride).toBe("Full prompt."); - expect(appendSystemPromptOverride).toEqual([]); - }); - - it("handles empty dynamic layer", () => { - const options = { - systemPrompt: "Stable.", - systemPromptLayers: { stable: "Stable.", dynamic: "" }, - }; - - const systemPromptOverride = - options.systemPromptLayers?.stable ?? options.systemPrompt; - const appendSystemPromptOverride = options.systemPromptLayers?.dynamic - ? [options.systemPromptLayers.dynamic] - : []; - - expect(systemPromptOverride).toBe("Stable."); - expect(appendSystemPromptOverride).toEqual([]); - }); -}); diff --git a/packages/engine/src/__tests__/prompt-cache-integration.test.ts b/packages/engine/src/__tests__/prompt-cache-integration.test.ts index 694852cbd..9e2b1e719 100644 --- a/packages/engine/src/__tests__/prompt-cache-integration.test.ts +++ b/packages/engine/src/__tests__/prompt-cache-integration.test.ts @@ -7,8 +7,9 @@ describe("cross-session prompt cache integration", () => { function simulateReviewerSession(sessionIndex: number): SystemPromptLayers { return buildPromptLayers({ - basePrompt: REVIEWER_SYSTEM_PROMPT + MEMORY_INSTRUCTIONS, + basePrompt: REVIEWER_SYSTEM_PROMPT, agentInstructions: `Session ${sessionIndex}: custom instructions that vary per agent.`, + memorySection: MEMORY_INSTRUCTIONS, pluginContributions: sessionIndex % 2 === 0 ? "## Plugin: lint\n\nCheck lint rules." : "", diff --git a/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts index 000ee086a..a45dbe774 100644 --- a/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts +++ b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts @@ -83,30 +83,29 @@ describe("prompt layers backward compatibility", () => { }); describe("reviewer assembly pattern", () => { - it("reproduces the reviewer prompt assembly", () => { + it("produces the expected reviewer prompt with memory in dynamic layer", () => { const basePrompt = "You are an independent code and plan reviewer."; const memoryInstructions = "\n## Memory\n\nUse fn_memory_search."; const agentInstructions = "Focus on security."; const plugins = "## Plugin: lint\n\nCheck eslint."; - // Old reviewer pattern: - // 1. buildSystemPromptWithInstructions(base + memory, instructions) - // 2. if plugins: concatenate - const oldSystemPrompt = buildSystemPromptWithInstructions( - basePrompt + memoryInstructions, - agentInstructions, - ); - const oldResult = `${oldSystemPrompt}\n\n${plugins}`; - - // New reviewer pattern: + // New reviewer pattern: memory goes in dynamic layer (not stable) so + // the stable prefix is byte-identical across sessions even when memory + // changes mid-task. const layers = buildPromptLayers({ - basePrompt: basePrompt + memoryInstructions, + basePrompt, agentInstructions, + memorySection: memoryInstructions, pluginContributions: plugins, }); - const newResult = collapsePromptLayers(layers); + const result = collapsePromptLayers(layers); - expect(newResult).toBe(oldResult); + // Base prompt is the stable layer + expect(layers.stable).toBe(basePrompt); + // Dynamic layer contains instructions, memory, and plugins + expect(result).toContain(agentInstructions); + expect(result).toContain("Memory"); + expect(result).toContain(plugins); }); }); diff --git a/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts b/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts index 54de4cb16..09694e15e 100644 --- a/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts +++ b/packages/engine/src/__tests__/reviewer-prompt-layers.test.ts @@ -5,26 +5,30 @@ describe("reviewer prompt layering", () => { const REVIEWER_BASE = "You are an independent code and plan reviewer."; const MEMORY_INSTRUCTIONS = "\n## Memory\n\nUse fn_memory_search to look up context."; - it("puts base prompt + memory instructions in stable layer", () => { + it("puts base prompt in stable layer and memory in dynamic layer", () => { const layers = buildPromptLayers({ - basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + basePrompt: REVIEWER_BASE, agentInstructions: "Custom reviewer guidance.", + memorySection: MEMORY_INSTRUCTIONS, pluginContributions: "## Plugin: lint\n\nCheck lint.", }); - expect(layers.stable).toBe(REVIEWER_BASE + MEMORY_INSTRUCTIONS); - expect(layers.stable).not.toContain("Custom reviewer guidance"); - expect(layers.stable).not.toContain("lint"); + expect(layers.stable).toBe(REVIEWER_BASE); + expect(layers.stable).not.toContain("Memory"); + expect(layers.dynamic).toContain("Memory"); + expect(layers.dynamic).toContain("Custom reviewer guidance"); }); it("produces identical stable layer across simulated sessions", () => { const layers1 = buildPromptLayers({ - basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + basePrompt: REVIEWER_BASE, agentInstructions: "Session 1 instructions.", + memorySection: MEMORY_INSTRUCTIONS, }); const layers2 = buildPromptLayers({ - basePrompt: REVIEWER_BASE + MEMORY_INSTRUCTIONS, + basePrompt: REVIEWER_BASE, agentInstructions: "Session 2 instructions.", + memorySection: MEMORY_INSTRUCTIONS, }); expect(layers1.stable).toBe(layers2.stable); diff --git a/packages/engine/src/__tests__/session-helpers-layers.test.ts b/packages/engine/src/__tests__/session-helpers-layers.test.ts deleted file mode 100644 index bf5e5cd79..000000000 --- a/packages/engine/src/__tests__/session-helpers-layers.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, it, expect } from "vitest"; -import type { ResolvedSessionOptions } from "../agent-session-helpers.js"; -import type { SystemPromptLayers } from "../prompt-layers.js"; - -describe("ResolvedSessionOptions layer forwarding", () => { - it("includes systemPromptLayers in the type", () => { - const layers: SystemPromptLayers = { - stable: "Stable prefix.", - dynamic: "Dynamic suffix.", - }; - - const options: Partial = { - systemPrompt: "Stable prefix.\n\nDynamic suffix.", - systemPromptLayers: layers, - }; - - expect(options.systemPromptLayers).toBeDefined(); - expect(options.systemPromptLayers!.stable).toBe("Stable prefix."); - }); -}); diff --git a/packages/engine/src/__tests__/skill-resolver-scoping.test.ts b/packages/engine/src/__tests__/skill-resolver-scoping.test.ts deleted file mode 100644 index f44d948df..000000000 --- a/packages/engine/src/__tests__/skill-resolver-scoping.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { getSkillPurposeFilter } from "../skill-resolver.js"; - -describe("getSkillPurposeFilter", () => { - it("returns a pass-all filter for executor sessions", () => { - const filter = getSkillPurposeFilter("executor"); - expect(filter("any-skill")).toBe(true); - expect(filter("fusion")).toBe(true); - expect(filter("deployment")).toBe(true); - }); - - it("filters to review-relevant skills for reviewer sessions", () => { - const filter = getSkillPurposeFilter("reviewer"); - expect(filter("code-review")).toBe(true); - expect(filter("security-review")).toBe(true); - expect(filter("review")).toBe(true); - expect(filter("deployment")).toBe(false); - expect(filter("fusion")).toBe(false); - }); - - it("returns a pass-all filter for unknown session purposes", () => { - const filter = getSkillPurposeFilter("unknown-purpose"); - expect(filter("any-skill")).toBe(true); - }); - - it("filters to minimal skills for heartbeat sessions", () => { - const filter = getSkillPurposeFilter("heartbeat"); - expect(filter("monitoring")).toBe(true); - expect(filter("heartbeat")).toBe(true); - expect(filter("fusion")).toBe(false); - expect(filter("code-review")).toBe(false); - }); - - it("returns a pass-all filter for triage sessions", () => { - const filter = getSkillPurposeFilter("triage"); - expect(filter("any-skill")).toBe(true); - expect(filter("fusion")).toBe(true); - }); -}); diff --git a/packages/engine/src/__tests__/skill-scoping-integration.test.ts b/packages/engine/src/__tests__/skill-scoping-integration.test.ts deleted file mode 100644 index 8aeafe1cb..000000000 --- a/packages/engine/src/__tests__/skill-scoping-integration.test.ts +++ /dev/null @@ -1,104 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { getSkillPurposeFilter } from "../skill-resolver.js"; - -/** - * Integration-style tests for getSkillPurposeFilter that verify filtering - * against a realistic set of skill names typical of a Fusion project. - * - * The existing skill-resolver-scoping.test.ts tests individual skill names - * in isolation. These tests verify the aggregate filtering behavior: how - * many skills each purpose loads from a full project skill set, and that - * the filter produces sensible subsets. - */ -describe("getSkillPurposeFilter integration", () => { - // Simulate a realistic set of skill names from a Fusion project - const ALL_SKILLS = [ - "fusion", - "code-review", - "security-review", - "review", - "deployment", - "task-management", - "monitoring", - "heartbeat", - "research", - "web-search", - ]; - - it("executor loads all skills", () => { - const filter = getSkillPurposeFilter("executor"); - const loaded = ALL_SKILLS.filter(filter); - expect(loaded).toEqual(ALL_SKILLS); - }); - - it("reviewer loads only review-related skills", () => { - const filter = getSkillPurposeFilter("reviewer"); - const loaded = ALL_SKILLS.filter(filter); - expect(loaded).toEqual(["code-review", "security-review", "review"]); - expect(loaded).not.toContain("fusion"); - expect(loaded).not.toContain("deployment"); - expect(loaded).not.toContain("monitoring"); - }); - - it("heartbeat loads only monitoring-related skills", () => { - const filter = getSkillPurposeFilter("heartbeat"); - const loaded = ALL_SKILLS.filter(filter); - expect(loaded).toEqual(["monitoring", "heartbeat"]); - expect(loaded).not.toContain("fusion"); - expect(loaded).not.toContain("code-review"); - }); - - it("triage loads all skills (same as executor)", () => { - const filter = getSkillPurposeFilter("triage"); - const loaded = ALL_SKILLS.filter(filter); - expect(loaded).toEqual(ALL_SKILLS); - }); - - it("reviewer filters out the majority of skills", () => { - const filter = getSkillPurposeFilter("reviewer"); - const loaded = ALL_SKILLS.filter(filter); - // Reviewer should load significantly fewer skills than total - expect(loaded.length).toBeLessThan(ALL_SKILLS.length / 2); - }); - - it("heartbeat filters to a minimal subset", () => { - const filter = getSkillPurposeFilter("heartbeat"); - const loaded = ALL_SKILLS.filter(filter); - // Heartbeat should have even fewer than reviewer - expect(loaded.length).toBeLessThanOrEqual(2); - }); - - it("unknown purpose passes all skills through (safe fallback)", () => { - const filter = getSkillPurposeFilter("some-future-purpose"); - const loaded = ALL_SKILLS.filter(filter); - expect(loaded).toEqual(ALL_SKILLS); - }); - - it("reviewer and heartbeat produce disjoint sets from the same input", () => { - const reviewerFilter = getSkillPurposeFilter("reviewer"); - const heartbeatFilter = getSkillPurposeFilter("heartbeat"); - const reviewerSkills = ALL_SKILLS.filter(reviewerFilter); - const heartbeatSkills = ALL_SKILLS.filter(heartbeatFilter); - - // No skill should appear in both reviewer and heartbeat sets - const overlap = reviewerSkills.filter((s) => heartbeatSkills.includes(s)); - expect(overlap).toEqual([]); - }); - - it("executor is a superset of all other purpose filters", () => { - const executorFilter = getSkillPurposeFilter("executor"); - const reviewerFilter = getSkillPurposeFilter("reviewer"); - const heartbeatFilter = getSkillPurposeFilter("heartbeat"); - - const executorSkills = new Set(ALL_SKILLS.filter(executorFilter)); - const reviewerSkills = ALL_SKILLS.filter(reviewerFilter); - const heartbeatSkills = ALL_SKILLS.filter(heartbeatFilter); - - for (const skill of reviewerSkills) { - expect(executorSkills.has(skill)).toBe(true); - } - for (const skill of heartbeatSkills) { - expect(executorSkills.has(skill)).toBe(true); - } - }); -}); diff --git a/packages/engine/src/__tests__/tool-ordering.test.ts b/packages/engine/src/__tests__/tool-ordering.test.ts deleted file mode 100644 index 1a54d8ba5..000000000 --- a/packages/engine/src/__tests__/tool-ordering.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { describe, it, expect } from "vitest"; - -/** - * Verifies that tools are sorted deterministically by name. - * This is critical for prompt caching — tool schemas are part of the - * API request, and reordering them breaks cache prefix matching. - */ -describe("deterministic tool ordering", () => { - it("sorts tools alphabetically by name", () => { - const tools = [ - { name: "write", execute: async () => {} }, - { name: "bash", execute: async () => {} }, - { name: "read", execute: async () => {} }, - { name: "edit", execute: async () => {} }, - ]; - - const sorted = [...tools].sort((a, b) => a.name.localeCompare(b.name)); - - expect(sorted.map((t) => t.name)).toEqual(["bash", "edit", "read", "write"]); - }); - - it("is stable across repeated sorts", () => { - const tools = [ - { name: "grep" }, - { name: "bash" }, - { name: "find" }, - { name: "read" }, - ]; - - const sorted1 = [...tools].sort((a, b) => a.name.localeCompare(b.name)); - const sorted2 = [...tools].sort((a, b) => a.name.localeCompare(b.name)); - - expect(sorted1.map((t) => t.name)).toEqual(sorted2.map((t) => t.name)); - }); -}); diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 06177eb41..55c04b53c 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -1787,7 +1787,7 @@ export async function createFnAgent(options: AgentOptions): Promise ...customToolList.map((tool) => tool.name), ...options.builtinToolsAllowlist, ]), - ]; + ].sort(); } return createAgentSession(createSessionOptions); diff --git a/packages/engine/src/reviewer.ts b/packages/engine/src/reviewer.ts index f0a2a473d..713ff7f7f 100644 --- a/packages/engine/src/reviewer.ts +++ b/packages/engine/src/reviewer.ts @@ -410,8 +410,9 @@ export async function reviewStep( : ""; // Build structured layers for cross-session prompt caching. - // The stable layer (base prompt + memory instructions) is byte-identical - // across all reviewer sessions in this task, enabling cache hits. + // The stable layer (base prompt only) is byte-identical across all + // reviewer sessions in this task, enabling cache hits. Memory goes + // into the dynamic layer because it can change between sessions. const reviewerPluginContributions = buildPluginPromptSection( "reviewer", options.pluginRunner, @@ -421,8 +422,9 @@ export async function reviewStep( } const layers = buildPromptLayers({ - basePrompt: reviewerBasePrompt + memorySection, + basePrompt: reviewerBasePrompt, agentInstructions: reviewerInstructions, + memorySection, pluginContributions: reviewerPluginContributions, }); diff --git a/packages/engine/src/session-token-usage.ts b/packages/engine/src/session-token-usage.ts index 3861fa4d4..fed43f2c7 100644 --- a/packages/engine/src/session-token-usage.ts +++ b/packages/engine/src/session-token-usage.ts @@ -92,13 +92,15 @@ export async function accumulateSessionTokenUsage( } /** - * Compute the cache hit ratio: the fraction of input tokens served from - * cache. Returns a number in [0, 1]. Useful for measuring the effectiveness - * of prompt caching optimizations. + * Compute the cache hit ratio: the fraction of effective input tokens served + * from cache. Returns a number in [0, 1]. Useful for measuring the + * effectiveness of prompt caching optimizations. * - * @param inputTokens - Non-cached input tokens (includes cache-write tokens) - * @param cachedTokens - Tokens read from cache - * @returns Cache hit ratio in [0, 1], or 0 if no tokens used + * @param inputTokens - Non-cached input tokens (NOT including cache-write tokens; + * use only `tokens.input` here, not `tokens.input + tokens.cacheWrite`) + * @param cachedTokens - Tokens read from cache (`tokens.cacheRead`) + * @returns Cache hit ratio in [0, 1] matching Anthropic console's `cache_read / input` metric, + * or 0 if no tokens used */ export function computeCacheHitRatio( inputTokens: number, diff --git a/packages/engine/src/skill-resolver.ts b/packages/engine/src/skill-resolver.ts index 5c5669cb1..cf106886c 100644 --- a/packages/engine/src/skill-resolver.ts +++ b/packages/engine/src/skill-resolver.ts @@ -485,45 +485,3 @@ export function createSkillsOverrideFromSelection( }; } -/** - * Skills that are relevant to each session purpose. Executors get all - * skills (they do the implementation work). Other roles get a scoped - * subset to avoid loading 71KB of reference material for every session. - * - * The filter matches against the skill's directory name (the last path - * segment before SKILL.md). - */ -const REVIEWER_SKILL_ALLOWLIST = new Set([ - "code-review", - "security-review", - "review", -]); - -const HEARTBEAT_SKILL_ALLOWLIST = new Set([ - "monitoring", - "heartbeat", -]); - -/** - * Return a filter function that decides whether a skill (by name) should - * be included in a session of the given purpose. - * - * - "executor" and "triage": all skills (pass-through) - * - "reviewer": only review-related skills - * - "heartbeat": only monitoring-related skills - * - unknown: all skills (safe fallback) - */ -export function getSkillPurposeFilter( - sessionPurpose: string, -): (skillName: string) => boolean { - switch (sessionPurpose) { - case "reviewer": - return (name) => REVIEWER_SKILL_ALLOWLIST.has(name); - case "heartbeat": - return (name) => HEARTBEAT_SKILL_ALLOWLIST.has(name); - case "executor": - case "triage": - default: - return () => true; - } -} From 2706a8384bfb56b2ad2ef7727d9123025813d055 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 12:44:32 -0400 Subject: [PATCH 12/16] fix(engine): preserve fn_heartbeat_done at end of tool list after sorting --- packages/engine/src/pi.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 55c04b53c..e9d16cb46 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -1760,7 +1760,14 @@ export async function createFnAgent(options: AgentOptions): Promise // Sort tools alphabetically by name for deterministic ordering. // Prompt caching requires the tool list to be byte-identical across // sessions — reordering breaks cache prefix matching. + // Exception: fn_heartbeat_done must remain last (stable terminal signal + // required by the heartbeat executor — see agent-heartbeat.ts). customToolList.sort((a, b) => a.name.localeCompare(b.name)); + const heartbeatDoneIdx = customToolList.findIndex((t) => t.name === "fn_heartbeat_done"); + if (heartbeatDoneIdx >= 0 && heartbeatDoneIdx < customToolList.length - 1) { + const [doneTool] = customToolList.splice(heartbeatDoneIdx, 1); + customToolList.push(doneTool); + } // Last-chance abort hook. Fires *here* — after every awaited setup step // in createFnAgent (provider registration, worktree validation, resource // loader reload) and immediately before the actual LLM session spawn. From cb4fb78d617f3143d7edb25908056813f9073e37 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 12:59:08 -0400 Subject: [PATCH 13/16] fix(engine): preserve memory-before-instructions ordering in dynamic layer --- .../prompt-layers-backward-compat.test.ts | 28 ++++++++++++------- .../src/__tests__/prompt-layers.test.ts | 2 +- packages/engine/src/prompt-layers.ts | 14 ++++++---- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts index a45dbe774..a323c2e69 100644 --- a/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts +++ b/packages/engine/src/__tests__/prompt-layers-backward-compat.test.ts @@ -83,15 +83,19 @@ describe("prompt layers backward compatibility", () => { }); describe("reviewer assembly pattern", () => { - it("produces the expected reviewer prompt with memory in dynamic layer", () => { + it("preserves memory-before-instructions ordering in dynamic layer", () => { const basePrompt = "You are an independent code and plan reviewer."; - const memoryInstructions = "\n## Memory\n\nUse fn_memory_search."; + const memoryInstructions = "## Memory\n\nUse fn_memory_search."; const agentInstructions = "Focus on security."; const plugins = "## Plugin: lint\n\nCheck eslint."; - // New reviewer pattern: memory goes in dynamic layer (not stable) so - // the stable prefix is byte-identical across sessions even when memory - // changes mid-task. + // The old reviewer pattern was: + // buildSystemPromptWithInstructions(base + memory, instructions) + plugins + // which produced: base + memory → instructions → plugins + // + // The new pattern moves memory from stable to dynamic (so stable prefix + // is byte-identical even if memory changes mid-task), but preserves the + // relative ordering: memory → instructions → plugins in the dynamic layer. const layers = buildPromptLayers({ basePrompt, agentInstructions, @@ -100,12 +104,16 @@ describe("prompt layers backward compatibility", () => { }); const result = collapsePromptLayers(layers); - // Base prompt is the stable layer + // Base prompt is the stable layer (no memory) expect(layers.stable).toBe(basePrompt); - // Dynamic layer contains instructions, memory, and plugins - expect(result).toContain(agentInstructions); - expect(result).toContain("Memory"); - expect(result).toContain(plugins); + + // Dynamic layer preserves: memory → instructions → plugins ordering + const memoryIdx = result.indexOf("## Memory"); + const instructionsIdx = result.indexOf("## Custom Instructions"); + const pluginsIdx = result.indexOf("## Plugin:"); + expect(memoryIdx).toBeGreaterThan(0); + expect(instructionsIdx).toBeGreaterThan(memoryIdx); + expect(pluginsIdx).toBeGreaterThan(instructionsIdx); }); }); diff --git a/packages/engine/src/__tests__/prompt-layers.test.ts b/packages/engine/src/__tests__/prompt-layers.test.ts index d34e96c9e..dd4eaf5b3 100644 --- a/packages/engine/src/__tests__/prompt-layers.test.ts +++ b/packages/engine/src/__tests__/prompt-layers.test.ts @@ -64,7 +64,7 @@ describe("buildPromptLayers", () => { }); expect(layers.dynamic).toBe( - "## Custom Instructions\n\nInstructions.\n\nMemory.\n\nPlugins." + "Memory.\n\n## Custom Instructions\n\nInstructions.\n\nPlugins." ); }); diff --git a/packages/engine/src/prompt-layers.ts b/packages/engine/src/prompt-layers.ts index 9e85e1c3b..4dd4ceddb 100644 --- a/packages/engine/src/prompt-layers.ts +++ b/packages/engine/src/prompt-layers.ts @@ -42,16 +42,20 @@ export function buildPromptLayers(input: PromptLayerInput): SystemPromptLayers { const dynamicParts: string[] = []; - const trimmedInstructions = agentInstructions?.trim() ?? ""; - if (trimmedInstructions) { - dynamicParts.push(`## Custom Instructions\n\n${trimmedInstructions}`); - } - + // Memory section comes before instructions to preserve the relative + // ordering from the legacy buildSystemPromptWithInstructions approach, + // where memory was concatenated onto basePrompt before instructions + // were appended. const trimmedMemory = memorySection?.trim() ?? ""; if (trimmedMemory) { dynamicParts.push(trimmedMemory); } + const trimmedInstructions = agentInstructions?.trim() ?? ""; + if (trimmedInstructions) { + dynamicParts.push(`## Custom Instructions\n\n${trimmedInstructions}`); + } + const trimmedPlugins = pluginContributions?.trim() ?? ""; if (trimmedPlugins) { dynamicParts.push(trimmedPlugins); From 9b6b22aa13f62816dfdb30aa415e3f0703f92974 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 13:57:11 -0400 Subject: [PATCH 14/16] =?UTF-8?q?fix(engine):=20remove=20leading=20newline?= =?UTF-8?q?=20from=20reviewer=20memorySection=20=E2=80=94=20buildPromptLay?= =?UTF-8?q?ers=20handles=20joining?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/engine/src/reviewer.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/reviewer.ts b/packages/engine/src/reviewer.ts index 713ff7f7f..d399e2539 100644 --- a/packages/engine/src/reviewer.ts +++ b/packages/engine/src/reviewer.ts @@ -405,8 +405,12 @@ export async function reviewStep( } } const reviewerBasePrompt = resolveAgentPrompt("reviewer", options.agentPrompts) || REVIEWER_SYSTEM_PROMPT; + // Memory goes in the dynamic layer (not concatenated onto basePrompt) so the + // stable prefix is byte-identical across sessions even if memory changes. + // The leading "\n" separator is no longer needed — buildPromptLayers handles + // section joining with "\n\n". const memorySection = options.rootDir && options.settings?.memoryEnabled !== false - ? "\n" + buildReviewerMemoryInstructions(options.rootDir, options.settings) + ? buildReviewerMemoryInstructions(options.rootDir, options.settings) : ""; // Build structured layers for cross-session prompt caching. From 74e2b0b69b83d264a5448d03cdb49b820cfb87a2 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 15:01:42 -0400 Subject: [PATCH 15/16] ci: retrigger CI (test shard 3/3 flaky failure) From 1880db08d4660c4170add8b11b01952ad4ff86f9 Mon Sep 17 00:00:00 2001 From: Matthew Greenberg Date: Sun, 10 May 2026 16:03:31 -0400 Subject: [PATCH 16/16] fix(engine): align computeCacheHitRatio JSDoc with stored tokenUsage format --- packages/engine/src/session-token-usage.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/session-token-usage.ts b/packages/engine/src/session-token-usage.ts index fed43f2c7..9d93f63ab 100644 --- a/packages/engine/src/session-token-usage.ts +++ b/packages/engine/src/session-token-usage.ts @@ -92,15 +92,13 @@ export async function accumulateSessionTokenUsage( } /** - * Compute the cache hit ratio: the fraction of effective input tokens served - * from cache. Returns a number in [0, 1]. Useful for measuring the - * effectiveness of prompt caching optimizations. + * Compute the cache hit ratio: `cachedTokens / (inputTokens + cachedTokens)`. + * Returns a number in [0, 1], or 0 when both arguments are 0. * - * @param inputTokens - Non-cached input tokens (NOT including cache-write tokens; - * use only `tokens.input` here, not `tokens.input + tokens.cacheWrite`) - * @param cachedTokens - Tokens read from cache (`tokens.cacheRead`) - * @returns Cache hit ratio in [0, 1] matching Anthropic console's `cache_read / input` metric, - * or 0 if no tokens used + * Compatible with stored `task.tokenUsage` fields: pass `inputTokens` (which + * includes cache-write tokens per `accumulateSessionTokenUsage`) and + * `cachedTokens` (cache-read tokens). Note this differs slightly from the + * Anthropic console metric, which excludes cache-write from the denominator. */ export function computeCacheHitRatio( inputTokens: number,