From d0018daad33e409720a5be18b9ddb4066565b972 Mon Sep 17 00:00:00 2001 From: Ramnique Singh <30795890+ramnique@users.noreply.github.com> Date: Fri, 10 Jul 2026 08:43:24 +0530 Subject: [PATCH] refactor(x): one ModeFlags schema behind all three composition shapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-toggled mode flags were hand-mirrored in three places (the resolver's CompositionOverrides zod schema, CapabilityContext, and ComposeSystemInstructionsInput) and copied field-by-field with '?? false' defaults — so a mode missed at one site compiled clean and silently never composed. Now capabilities/types.ts declares one zod ModeFlags schema with defaults: the wire shape (all-optional, what turn files carry), the resolver's concrete parse output, and the composition context all derive from it; CompositionOverrides just extends it with the resolver-only keys, the composer takes {instructions, notes, workDir} & ModeFlags with a rest-spread (no copy list), and the legacy call site passes explicit false for the modes it never composes. Adding a mode = one schema key + one record; anything less is a compile/parse error. New resolver test pins historical-composition compatibility (sparse, null-heavy, unknown-key, and garbage compositions all compose exactly as before). Golden snapshots unchanged. Co-Authored-By: Claude Fable 5 --- .../core/src/agents/compose-instructions.ts | 50 +++++++------------ apps/x/packages/core/src/agents/runtime.ts | 3 ++ .../assistant/capabilities/types.ts | 39 ++++++++++----- .../turns/bridges/real-agent-resolver.test.ts | 40 +++++++++++++++ .../src/turns/bridges/real-agent-resolver.ts | 38 ++++++-------- 5 files changed, 102 insertions(+), 68 deletions(-) diff --git a/apps/x/packages/core/src/agents/compose-instructions.ts b/apps/x/packages/core/src/agents/compose-instructions.ts index a5d925c7..3a43c964 100644 --- a/apps/x/packages/core/src/agents/compose-instructions.ts +++ b/apps/x/packages/core/src/agents/compose-instructions.ts @@ -3,7 +3,10 @@ import { AGENT_NOTES_CAPABILITY, WORK_DIRECTORY_CAPABILITY, } from "../application/assistant/capabilities/workspace.js"; -import type { CapabilityContext } from "../application/assistant/capabilities/types.js"; +import type { + CapabilityContext, + ModeFlags, +} from "../application/assistant/capabilities/types.js"; // Everything that composes into the system prompt, in composition order. const PROMPT_CAPABILITIES = [ @@ -33,50 +36,31 @@ If Middle pane state is note, the supplied path and content are available so you If Middle pane state is browser, only the URL and page title are supplied; the page content itself is NOT included. If you need the page content to answer, use the browser tools available to you to read the page. The user may or may not be talking about this page. Only reference or act on this page when the user's message clearly relates to it, such as "this page", "this article", "what I'm looking at", "this site", or "summarize this". For unrelated questions, ignore this page entirely and answer normally. Do not mention that you can see the browser unless it is relevant to the answer.`; -export interface ComposeSystemInstructionsInput { +// The mode flags come straight from the shared ModeFlags shape (all +// required — callers normalize via ModeFlags.parse or pass explicit +// values), so a mode added to the schema is a compile error at every call +// site until it is threaded through, never a silently-absent prompt block. +export type ComposeSystemInstructionsInput = { instructions: string; agentNotesContext: string | null; userWorkDir: string | null; - voiceInput: boolean; - voiceOutput: 'summary' | 'full' | null; - searchEnabled: boolean; - codeMode: 'claude' | 'codex' | null; - codeCwd: string | null; - // Optional so legacy callers (old streamAgent path) are unaffected. - videoMode?: boolean; - coachMode?: boolean; -} +} & ModeFlags; -// System-prompt assembly, extracted verbatim from streamAgent so the new turn -// runtime's agent resolver composes byte-identical prompts. Pure: callers -// load agent notes / work dir themselves. +// System-prompt assembly: base instructions + hidden-user-context + the +// capability fragments. Pure: callers load agent notes / work dir +// themselves. export function composeSystemInstructions({ instructions, agentNotesContext, userWorkDir, - voiceInput, - voiceOutput, - searchEnabled, - codeMode, - codeCwd, - videoMode, - coachMode, + ...modeFlags }: ComposeSystemInstructionsInput): string { let composed = `${instructions}\n\n${USER_CONTEXT_SYSTEM_INSTRUCTIONS}`; // Capabilities compose in PROMPT_CAPABILITIES order — a fixed total // order, so identical inputs yield identical bytes. The fragment text - // lives with the capability records. - const ctx: CapabilityContext = { - agentNotesContext, - userWorkDir, - voiceInput, - voiceOutput, - searchEnabled, - codeMode, - codeCwd, - videoMode: videoMode ?? false, - coachMode: coachMode ?? false, - }; + // lives with the capability records; the rest-spread means new schema + // keys flow through without a hand-maintained copy list. + const ctx: CapabilityContext = { agentNotesContext, userWorkDir, ...modeFlags }; for (const capability of PROMPT_CAPABILITIES) { const fragment = capability.promptFragment(ctx); if (fragment) { diff --git a/apps/x/packages/core/src/agents/runtime.ts b/apps/x/packages/core/src/agents/runtime.ts index ac68c3ee..53534fd0 100644 --- a/apps/x/packages/core/src/agents/runtime.ts +++ b/apps/x/packages/core/src/agents/runtime.ts @@ -1322,6 +1322,9 @@ export async function* streamAgent({ searchEnabled, codeMode, codeCwd, + // The legacy runs engine never composes video/coach modes. + videoMode: false, + coachMode: false, }); let streamError: string | null = null; for await (const event of streamLlm( diff --git a/apps/x/packages/core/src/application/assistant/capabilities/types.ts b/apps/x/packages/core/src/application/assistant/capabilities/types.ts index 033be196..1472a428 100644 --- a/apps/x/packages/core/src/application/assistant/capabilities/types.ts +++ b/apps/x/packages/core/src/application/assistant/capabilities/types.ts @@ -23,25 +23,38 @@ // injecting into every turn's system prompt would be a standing // prompt-injection channel. +import { z } from "zod"; + export type CapabilityActivation = "model" | "app" | "always"; -// Inputs an eager fragment may read. All fields mirror persisted -// RequestedAgent.overrides.composition values, resolved before composition — -// a fragment must be a pure function of this context so composed prompts -// stay byte-identical for identical inputs (provider prefix caching and -// agent-snapshot inheritance both depend on it). -export interface CapabilityContext { +// THE single source of truth for the app-toggled mode flags. Every key is a +// persisted RequestedAgent.overrides.composition value; defaults make the +// parsed output fully concrete, so the wire shape (all-optional, what turn +// files carry), the resolver's parse result, and the composition context are +// one declaration. Adding a mode = one key here + one record in modes.ts; +// forgetting anything else is a compile error, not a silently-absent prompt +// block. Every key alters system-prompt bytes, so prompt-affecting inputs +// must stay session-sticky (prefix caching). +export const ModeFlags = z.object({ + voiceInput: z.boolean().default(false), + voiceOutput: z.enum(["summary", "full"]).nullable().default(null), + searchEnabled: z.boolean().default(false), + codeMode: z.enum(["claude", "codex"]).nullable().default(null), + codeCwd: z.string().nullable().default(null), + videoMode: z.boolean().default(false), + coachMode: z.boolean().default(false), +}); +export type ModeFlags = z.infer; + +// Inputs an eager fragment may read: the mode flags plus resolved workspace +// context. A fragment must be a pure function of this context so composed +// prompts stay byte-identical for identical inputs (provider prefix caching +// and agent-snapshot inheritance both depend on it). +export interface CapabilityContext extends ModeFlags { // Workspace context (workspaceContext trait agents only; the resolver // loads these and leaves them null for everyone else). agentNotesContext: string | null; userWorkDir: string | null; - voiceInput: boolean; - voiceOutput: "summary" | "full" | null; - searchEnabled: boolean; - codeMode: "claude" | "codex" | null; - codeCwd: string | null; - videoMode: boolean; - coachMode: boolean; } // Model-activated (a "skill"): advertised in the loadSkill catalog, loaded diff --git a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts index 1f7f46ec..0f0e090f 100644 --- a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts +++ b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.test.ts @@ -289,3 +289,43 @@ describe("active skills (skill-scoped tools)", () => { expect(resolved.tools).toEqual([]); }); }); + +describe("historical composition compatibility", () => { + // Persisted RequestedAgent.overrides.composition values are replayed + // from turn files; the ModeFlags-derived schema must keep accepting + // every historical shape byte-for-byte. + it("composes identically for sparse, null-heavy, and unknown-key compositions", async () => { + const resolver = makeResolver(makeAgent()); + const sparse = await resolver.resolve({ + agentId: "copilot", + overrides: { composition: { voiceInput: true } }, + }); + const explicit = await resolver.resolve({ + agentId: "copilot", + overrides: { + composition: { + voiceInput: true, + voiceOutput: null, + searchEnabled: false, + codeMode: null, + codeCwd: null, + videoMode: false, + coachMode: false, + workDirId: null, + // Unknown keys have always been ignored. + futureUnknownKey: "ignored", + }, + }, + }); + expect(sparse.systemPrompt).toBe(explicit.systemPrompt); + expect(sparse.systemPrompt).toContain("# Voice Input"); + + // Garbage compositions fall back to all-defaults, not a throw. + const garbage = await resolver.resolve({ + agentId: "copilot", + overrides: { composition: { voiceInput: "yes" } }, + }); + const none = await resolver.resolve({ agentId: "copilot" }); + expect(garbage.systemPrompt).toBe(none.systemPrompt); + }); +}); diff --git a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts index 24dd787e..6992e958 100644 --- a/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts +++ b/apps/x/packages/core/src/turns/bridges/real-agent-resolver.ts @@ -14,6 +14,7 @@ import { import { hasWorkspaceContext, loadAgent } from "../../agents/registry.js"; import { BuiltinTools } from "../../application/lib/builtin-tools.js"; import { skillToolNames } from "../../application/assistant/skills/index.js"; +import { ModeFlags } from "../../application/assistant/capabilities/types.js"; import { getDefaultModelAndProvider } from "../../models/defaults.js"; import { builtinToolDescriptor, @@ -51,15 +52,11 @@ const ASK_HUMAN_DESCRIPTOR: z.infer = { // Unknown keys are ignored. Prompt-affecting inputs should be session-sticky: // every key here alters system-prompt bytes and therefore busts provider // prefix caching when it changes between turns. -const CompositionOverrides = z.object({ +// The mode-flag keys come from the shared ModeFlags schema (the single +// source of truth in capabilities/types.ts — defaults make the parse output +// fully concrete); only the resolver-specific keys are declared here. +const CompositionOverrides = ModeFlags.extend({ workDirId: z.string().nullable().optional(), - voiceInput: z.boolean().optional(), - voiceOutput: z.enum(["summary", "full"]).nullable().optional(), - searchEnabled: z.boolean().optional(), - codeMode: z.enum(["claude", "codex"]).nullable().optional(), - codeCwd: z.string().nullable().optional(), - videoMode: z.boolean().optional(), - coachMode: z.boolean().optional(), // Set by spawn-agent for by-id children: strips the spawn tool so depth // is capped at 1 regardless of which stored agent is spawned. subagent: z.boolean().optional(), @@ -123,7 +120,12 @@ export class RealAgentResolver { const parsed = CompositionOverrides.safeParse( requested.overrides?.composition ?? {}, ); - const composition = parsed.success ? parsed.data : {}; + // An unparseable composition falls back to all defaults (same shape + // as parsing {}), preserving the historical "ignore garbage" rule. + const composition = parsed.success + ? parsed.data + : CompositionOverrides.parse({}); + const { workDirId, subagent, activeSkills, ...modeFlags } = composition; // Agent notes and work-dir context are scoped to agents with the // workspaceContext trait (the copilot), per the agent registry. const copilotContext = hasWorkspaceContext(requested.agentId); @@ -131,23 +133,15 @@ export class RealAgentResolver { instructions: agent.instructions, agentNotesContext: copilotContext ? this.loadNotes() : null, userWorkDir: - copilotContext && composition.workDirId - ? this.loadWorkDir(composition.workDirId) - : null, - voiceInput: composition.voiceInput ?? false, - voiceOutput: composition.voiceOutput ?? null, - searchEnabled: composition.searchEnabled ?? false, - codeMode: composition.codeMode ?? null, - codeCwd: composition.codeCwd ?? null, - videoMode: composition.videoMode ?? false, - coachMode: composition.coachMode ?? false, + copilotContext && workDirId ? this.loadWorkDir(workDirId) : null, + ...modeFlags, }); const tools = await this.resolveTools(agent, { - subagent: composition.subagent ?? false, + subagent: subagent ?? false, }); - if (copilotContext && composition.activeSkills?.length) { - await this.appendSkillTools(tools, composition.activeSkills); + if (copilotContext && activeSkills?.length) { + await this.appendSkillTools(tools, activeSkills); } return ResolvedAgent.parse({ agentId: requested.agentId,