diff --git a/src/resources/extensions/sf/agents/rubber-duck.agent.yaml b/src/resources/extensions/sf/agents/rubber-duck.agent.yaml index 258bab91e..86d00cf4b 100644 --- a/src/resources/extensions/sf/agents/rubber-duck.agent.yaml +++ b/src/resources/extensions/sf/agents/rubber-duck.agent.yaml @@ -8,17 +8,10 @@ description: > pipeline (rubber-duck reviews triage decisions before they auto-apply), but operators can invoke directly for any artifact review. tools: "*" -# promptParts mirrors copilot's declarative composition matrix. SF doesn't -# yet honor these flags at runtime — they document INTENT so the day the -# prompt-composition runtime lands, this agent picks it up automatically -# without a YAML edit. Today's effective behavior is: the full `prompt:` -# below is used verbatim. promptParts: - includeAISafety: true - includeToolInstructions: true - includeParallelToolCalling: true - includeCustomAgentInstructions: false - includeEnvironmentContext: false + - aiSafety + - toolInstructions + - parallelToolCalling prompt: | You are SF's rubber-duck agent: a disciplined devil's advocate. You review artifacts and surface only issues that genuinely matter. diff --git a/src/resources/extensions/sf/subagent/agents.js b/src/resources/extensions/sf/subagent/agents.js index 9ad8074b7..13e49371e 100644 --- a/src/resources/extensions/sf/subagent/agents.js +++ b/src/resources/extensions/sf/subagent/agents.js @@ -5,8 +5,31 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { getAgentDir, parseFrontmatter } from "@singularity-forge/coding-agent"; import { parse as parseYaml } from "yaml"; +import { validatePromptParts } from "./prompt-parts.js"; const PROJECT_AGENT_DIR_CANDIDATES = [".sf", ".pi"]; + +export function validateAgentDefinition(definition) { + const errors = []; + if (typeof definition?.name !== "string" || !definition.name.trim()) { + errors.push("name must be a non-empty string"); + } + if ( + typeof definition?.description !== "string" || + !definition.description.trim() + ) { + errors.push("description must be a non-empty string"); + } + if ( + typeof definition?.systemPrompt !== "string" || + !definition.systemPrompt.trim() + ) { + errors.push("prompt must be a non-empty string"); + } + errors.push(...validatePromptParts(definition?.promptParts)); + return errors; +} + export function parseConflictsWith(value) { if (typeof value !== "string") return undefined; const conflicts = value @@ -99,11 +122,13 @@ function loadAgentsFromDir(dir, source) { } catch { continue; } - if ( - typeof definition.name !== "string" || - typeof definition.description !== "string" || - typeof definition.systemPrompt !== "string" - ) { + const validationErrors = validateAgentDefinition(definition); + if (validationErrors.length > 0) { + if (source === "builtin") { + throw new Error( + `Invalid built-in agent ${filePath}: ${validationErrors.join("; ")}`, + ); + } continue; } const tools = parseAgentTools(definition.tools); diff --git a/src/resources/extensions/sf/subagent/index.js b/src/resources/extensions/sf/subagent/index.js index b3dd3171f..338441d1a 100644 --- a/src/resources/extensions/sf/subagent/index.js +++ b/src/resources/extensions/sf/subagent/index.js @@ -45,6 +45,7 @@ import { mergeDeltaPatches, readIsolationMode, } from "./isolation.js"; +import { composeAgentPrompt } from "./prompt-parts.js"; import { registerWorker, updateWorker } from "./worker-registry.js"; const MAX_PARALLEL_TASKS = 8; @@ -1209,7 +1210,14 @@ async function runSingleAgent( }; try { if (agent.systemPrompt.trim()) { - const tmp = writePromptToTempFile(agent.name, agent.systemPrompt); + const tmp = writePromptToTempFile( + agent.name, + composeAgentPrompt(agent, { + cwd: cwd ?? defaultCwd, + surface: "subagent", + tools: agent.tools, + }), + ); tmpPromptDir = tmp.dir; tmpPromptPath = tmp.filePath; } diff --git a/src/resources/extensions/sf/subagent/prompt-parts.js b/src/resources/extensions/sf/subagent/prompt-parts.js new file mode 100644 index 000000000..28dbc8179 --- /dev/null +++ b/src/resources/extensions/sf/subagent/prompt-parts.js @@ -0,0 +1,165 @@ +/** + * prompt-parts.js — declarative prompt section registry for SF agents. + * + * Purpose: make `.agent.yaml` promptParts executable instead of decorative: + * agents declare which standard sections they need, while this module owns + * normalization, validation, ordering, and composition. + * + * Consumer: subagent agent discovery, subagent process launch, and headless + * triage apply when composing built-in agent prompts. + */ + +/** + * Render named, reusable sections for agent prompt composition. + * + * Purpose: keep `.agent.yaml` promptParts declarations backed by one executable + * registry instead of copying prompt boilerplate into every agent definition. + * + * Consumer: composeAgentPrompt and prompt-parts validation tests. + */ +export const PROMPT_PARTS = { + aiSafety: { + label: "AI Safety", + render: () => + "Follow the active safety and execution policies. Refuse unsafe requests and surface uncertainty instead of fabricating evidence.", + }, + toolInstructions: { + label: "Tool Instructions", + render: (ctx = {}) => { + const tools = Array.isArray(ctx.tools) ? ctx.tools.filter(Boolean) : []; + if (tools.length === 0) { + return "Use only the tools exposed by the runtime for this agent."; + } + return `Available tools for this agent: ${tools.join(", ")}. Stay within this surface.`; + }, + }, + parallelToolCalling: { + label: "Parallel Tool Calling", + render: () => + "When independent read-only checks are needed, batch them in parallel. Do not parallelize dependent or mutating steps.", + }, + customAgentInstructions: { + label: "Custom Agent Instructions", + render: (ctx = {}) => + typeof ctx.customAgentInstructions === "string" + ? ctx.customAgentInstructions + : "", + }, + environmentContext: { + label: "Environment Context", + render: (ctx = {}) => { + const rows = []; + if (ctx.cwd) rows.push(`Working directory: ${ctx.cwd}`); + if (ctx.surface) rows.push(`Surface: ${ctx.surface}`); + return rows.join("\n"); + }, + }, + agentBody: { + label: "Agent Instructions", + render: (ctx = {}) => ctx.agentBody ?? "", + }, + outputContract: { + label: "Output Contract", + render: (ctx = {}) => + typeof ctx.outputContract === "string" ? ctx.outputContract : "", + }, +}; + +/** + * Define the canonical order for reusable prompt sections. + * + * Purpose: make YAML declaration order non-semantic so agents cannot create + * inconsistent safety/tool/body ordering by accident. + * + * Consumer: normalizePromptParts and composeAgentPrompt. + */ +export const PROMPT_PART_ORDER = [ + "aiSafety", + "toolInstructions", + "parallelToolCalling", + "customAgentInstructions", + "environmentContext", + "agentBody", + "outputContract", +]; + +/** + * Provide the valid prompt part key set for schema-like validation. + * + * Purpose: reject stale Copilot-style boolean fields and misspelled named parts + * before an agent prompt reaches runtime. + * + * Consumer: validatePromptParts. + */ +export const KNOWN_PROMPT_PART_KEYS = new Set(PROMPT_PART_ORDER); + +/** + * Normalize a promptParts declaration into canonical execution order. + * + * Purpose: let agents declare the named parts they need while guaranteeing that + * the agent body is still included and ordered consistently. + * + * Consumer: composeAgentPrompt. + */ +export function normalizePromptParts(value) { + if (value === undefined) return null; + if (Array.isArray(value)) { + const enabled = new Set(); + for (const key of value) { + if (typeof key === "string" && PROMPT_PARTS[key]) enabled.add(key); + } + enabled.add("agentBody"); + return PROMPT_PART_ORDER.filter((key) => enabled.has(key)); + } + return null; +} + +/** + * Validate a promptParts declaration. + * + * Purpose: make the canonical named-array shape enforceable for built-in, + * project, and user agent definitions. + * + * Consumer: validateAgentDefinition and validateManifestPromptParts. + */ +export function validatePromptParts(value) { + const errors = []; + if (value === undefined) return errors; + if (!Array.isArray(value)) { + return ["promptParts must be an array of canonical part names"]; + } + for (const [idx, key] of value.entries()) { + if (typeof key !== "string") { + errors.push(`promptParts[${idx}] must be a string`); + } else if (!KNOWN_PROMPT_PART_KEYS.has(key)) { + errors.push(`promptParts[${idx}] "${key}" is not a known prompt part`); + } + } + return errors; +} + +/** + * Compose an agent prompt from named registry parts plus the agent body. + * + * Purpose: make promptParts declarations executable on every subagent launch + * while preserving raw prompts for definitions that do not opt in. + * + * Consumer: subagent process launch and headless triage apply. + */ +export function composeAgentPrompt(agent, context = {}) { + const body = agent?.systemPrompt ?? ""; + const partKeys = normalizePromptParts(agent?.promptParts); + if (!partKeys) return body; + const blocks = []; + for (const key of partKeys) { + const part = PROMPT_PARTS[key]; + if (!part) continue; + const text = part.render({ + ...context, + agentBody: body, + }); + if (typeof text !== "string" || text.trim().length === 0) continue; + blocks.push(`## ${part.label}\n\n${text.trim()}`); + } + return blocks.join("\n\n"); +} diff --git a/src/resources/extensions/sf/tests/subagent-agent-yaml.test.mjs b/src/resources/extensions/sf/tests/subagent-agent-yaml.test.mjs index b7e910d7d..d7996cca0 100644 --- a/src/resources/extensions/sf/tests/subagent-agent-yaml.test.mjs +++ b/src/resources/extensions/sf/tests/subagent-agent-yaml.test.mjs @@ -4,7 +4,12 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "vitest"; -import { discoverAgents } from "../subagent/agents.js"; +import { discoverAgents, validateAgentDefinition } from "../subagent/agents.js"; +import { + composeAgentPrompt, + normalizePromptParts, + validatePromptParts, +} from "../subagent/prompt-parts.js"; function makeProject() { const project = mkdtempSync(join(tmpdir(), "sf-agent-yaml-")); @@ -27,8 +32,8 @@ test("discoverAgents_when_project_has_copilot_style_agent_yaml_loads_agent", () " - glob", " - view", "promptParts:", - " includeAISafety: true", - " includeEnvironmentContext: false", + " - aiSafety", + " - toolInstructions", "prompt: |", " You are an exploration agent.", " Answer quickly and cite exact files.", @@ -48,10 +53,7 @@ test("discoverAgents_when_project_has_copilot_style_agent_yaml_loads_agent", () ); assert.equal(agent.model, "claude-haiku-4.5"); assert.deepEqual(agent.tools, ["grep", "glob", "view"]); - assert.deepEqual(agent.promptParts, { - includeAISafety: true, - includeEnvironmentContext: false, - }); + assert.deepEqual(agent.promptParts, ["aiSafety", "toolInstructions"]); assert.match(agent.systemPrompt, /You are an exploration agent/); }); @@ -136,11 +138,11 @@ test("discoverAgents_when_scope_both_includes_builtin_rubber_duck_and_triage_dec "triage-decider builtin agent must be discovered in scope=both", ); assert.equal(triageDecider.source, "builtin"); - // triage-decider has resolve_issue in its tools allowlist — that's the - // key capability that lets it close entries autonomously. + // triage-decider is plan-only; the apply runner owns mutation after + // rubber-duck agrees. assert.ok( - triageDecider.tools?.includes("resolve_issue"), - "triage-decider must declare resolve_issue tool access", + !triageDecider.tools?.includes("resolve_issue"), + "triage-decider must not declare resolve_issue tool access", ); } finally { if (originalEnv === undefined) delete process.env.SF_CODING_AGENT_DIR; @@ -177,3 +179,110 @@ test("discoverAgents_when_project_overrides_builtin_name_project_wins", () => { else process.env.SF_CODING_AGENT_DIR = originalEnv; } }); + +test("validateAgentDefinition_when_builtin_promptParts_unknown_name_reports_error", () => { + const errors = validateAgentDefinition({ + name: "bad-agent", + description: "Bad agent", + systemPrompt: "Do the thing.", + promptParts: ["aiSafety", "includeInventedFuturePart"], + }); + + assert.ok( + errors.some((error) => + error.includes('"includeInventedFuturePart" is not a known prompt part'), + ), + ); +}); + +test("validatePromptParts_when_legacy_boolean_shape_is_used_rejects_it", () => { + assert.deepEqual(validatePromptParts({ includeAISafety: true }), [ + "promptParts must be an array of canonical part names", + ]); +}); + +test("composeAgentPrompt_when_prompt_parts_absent_preserves_raw_prompt", () => { + assert.equal( + composeAgentPrompt({ + name: "plain", + description: "plain", + systemPrompt: "Plain prompt.", + }), + "Plain prompt.", + ); +}); + +test("composeAgentPrompt_when_prompt_parts_present_uses_registry_order", () => { + const prompt = composeAgentPrompt( + { + name: "explore", + description: "Explore.", + systemPrompt: "Agent body.", + promptParts: ["toolInstructions", "aiSafety"], + tools: ["grep", "view"], + }, + { tools: ["grep", "view"] }, + ); + + assert.match(prompt, /## AI Safety/); + assert.match(prompt, /## Tool Instructions/); + assert.match(prompt, /## Agent Instructions/); + assert.ok( + prompt.indexOf("## AI Safety") < prompt.indexOf("## Tool Instructions"), + ); + assert.ok( + prompt.indexOf("## Tool Instructions") < + prompt.indexOf("## Agent Instructions"), + ); + assert.match(prompt, /Available tools for this agent: grep, view/); + assert.match(prompt, /Agent body\./); +}); + +test("composeAgentPrompt_when_output_contract_declared_appends_contract_section", () => { + const prompt = composeAgentPrompt( + { + name: "contracted", + description: "Contracted.", + systemPrompt: "Agent body.", + promptParts: ["outputContract"], + }, + { outputContract: "Return JSON only." }, + ); + + assert.match(prompt, /## Agent Instructions/); + assert.match(prompt, /Agent body\./); + assert.match(prompt, /## Output Contract/); + assert.match(prompt, /Return JSON only\./); + assert.ok( + prompt.indexOf("## Agent Instructions") < + prompt.indexOf("## Output Contract"), + ); +}); + +test("normalizePromptParts_when_given_canonical_names_always_includes_agent_body", () => { + assert.deepEqual(normalizePromptParts(["toolInstructions", "aiSafety"]), [ + "aiSafety", + "toolInstructions", + "agentBody", + ]); +}); + +test("discoverAgents_when_scope_both_validates_builtin_promptParts_contract", () => { + const isolatedAgentDir = mkdtempSync(join(tmpdir(), "sf-agent-dir-")); + const originalEnv = process.env.SF_CODING_AGENT_DIR; + process.env.SF_CODING_AGENT_DIR = isolatedAgentDir; + try { + const project = makeProject(); + const { agents } = discoverAgents(project, "both"); + for (const agent of agents.filter((entry) => entry.source === "builtin")) { + assert.deepEqual( + validateAgentDefinition(agent), + [], + `${agent.name} built-in agent should validate`, + ); + } + } finally { + if (originalEnv === undefined) delete process.env.SF_CODING_AGENT_DIR; + else process.env.SF_CODING_AGENT_DIR = originalEnv; + } +}); diff --git a/src/resources/extensions/sf/tests/unit-context-manifest-prompt-parts.test.mjs b/src/resources/extensions/sf/tests/unit-context-manifest-prompt-parts.test.mjs new file mode 100644 index 000000000..8996e9d31 --- /dev/null +++ b/src/resources/extensions/sf/tests/unit-context-manifest-prompt-parts.test.mjs @@ -0,0 +1,27 @@ +import assert from "node:assert/strict"; +import { test } from "vitest"; + +import { + resolveManifest, + validateManifestPromptParts, +} from "../unit-context-manifest.js"; + +test("unit_manifest_promptParts_when_declared_use_canonical_part_names", () => { + const manifest = resolveManifest("plan-milestone"); + + assert.deepEqual(manifest.promptParts, [ + "aiSafety", + "toolInstructions", + "environmentContext", + ]); + assert.deepEqual(validateManifestPromptParts(manifest), []); +}); + +test("unit_manifest_promptParts_when_legacy_boolean_shape_is_used_rejects_it", () => { + assert.deepEqual( + validateManifestPromptParts({ + promptParts: { includeAISafety: true }, + }), + ["promptParts must be an array of canonical part names"], + ); +}); diff --git a/src/resources/extensions/sf/unit-context-manifest.js b/src/resources/extensions/sf/unit-context-manifest.js index 7c3c358bf..8f1e69884 100644 --- a/src/resources/extensions/sf/unit-context-manifest.js +++ b/src/resources/extensions/sf/unit-context-manifest.js @@ -26,6 +26,8 @@ // the composer's job; manifests describe intent, not disk layout. // - Char budgets are nominal — blown budgets log a telemetry event, // they do not truncate or error (the composer decides fallback). +import { validatePromptParts } from "./subagent/prompt-parts.js"; + // ─── Artifact registry ──────────────────────────────────────────────────── /** * Stable identifiers for every artifact class a unit might inline, excerpt, @@ -157,6 +159,7 @@ export const UNIT_MANIFESTS = { maxSystemPromptChars: COMMON_BUDGET_MEDIUM, }, "plan-milestone": { + promptParts: ["aiSafety", "toolInstructions", "environmentContext"], skills: { mode: "all" }, knowledge: "full", memory: "prompt-relevant", @@ -579,7 +582,7 @@ export const UNIT_MANIFESTS = { maxSystemPromptChars: COMMON_BUDGET_MEDIUM, }, // ─── Ops / deployment pipeline ──────────────────────────────────────── - "deploy": { + deploy: { skills: { mode: "all" }, knowledge: "critical-only", memory: "critical-only", @@ -607,7 +610,7 @@ export const UNIT_MANIFESTS = { }, maxSystemPromptChars: COMMON_BUDGET_SMALL, }, - "release": { + release: { skills: { mode: "all" }, knowledge: "critical-only", memory: "critical-only", @@ -621,7 +624,7 @@ export const UNIT_MANIFESTS = { }, maxSystemPromptChars: COMMON_BUDGET_SMALL, }, - "rollback": { + rollback: { skills: { mode: "all" }, knowledge: "critical-only", memory: "critical-only", @@ -635,7 +638,7 @@ export const UNIT_MANIFESTS = { }, maxSystemPromptChars: COMMON_BUDGET_SMALL, }, - "challenge": { + challenge: { skills: { mode: "all" }, knowledge: "critical-only", memory: "critical-only", @@ -661,3 +664,7 @@ export const UNIT_MANIFESTS = { export function resolveManifest(unitType) { return UNIT_MANIFESTS[unitType] ?? null; } + +export function validateManifestPromptParts(manifest) { + return validatePromptParts(manifest?.promptParts); +}