mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-07-12 21:02:17 +02:00
refactor(x): one ModeFlags schema behind all three composition shapes
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 <noreply@anthropic.com>
This commit is contained in:
parent
8cf1335577
commit
d0018daad3
5 changed files with 102 additions and 68 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<typeof ModeFlags>;
|
||||
|
||||
// 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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<typeof ToolDescriptor> = {
|
|||
// 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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue